Message ID | 20211105033558.1573552-1-bryan.odonoghue@linaro.org |
---|---|
Headers | show |
Series | Add pm8150b TPCM driver | expand |
On Fri, Nov 05, 2021 at 03:35:57AM +0000, Bryan O'Donoghue wrote: > Remove the standalone PMIC Type-C driver. We have implemented a full TCPM > driver which covers and extends the functionality in this driver. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > drivers/usb/typec/Kconfig | 13 -- > drivers/usb/typec/Makefile | 1 - > drivers/usb/typec/qcom-pmic-typec.c | 262 ---------------------------- > 3 files changed, 276 deletions(-) > delete mode 100644 drivers/usb/typec/qcom-pmic-typec.c > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig > index ab480f38523aa..61fba7bd1671d 100644 > --- a/drivers/usb/typec/Kconfig > +++ b/drivers/usb/typec/Kconfig > @@ -75,19 +75,6 @@ config TYPEC_STUSB160X > If you choose to build this driver as a dynamically linked module, the > module will be called stusb160x.ko. > > -config TYPEC_QCOM_PMIC > - tristate "Qualcomm PMIC USB Type-C driver" > - depends on ARCH_QCOM || COMPILE_TEST > - depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > - help > - Driver for supporting role switch over the Qualcomm PMIC. This will > - handle the USB Type-C role and orientation detection reported by the > - QCOM PMIC if the PMIC has the capability to handle USB Type-C > - detection. > - > - It will also enable the VBUS output to connected devices when a > - DFP connection is made. I don't like that you create point where the support is temporarily removed for this hardware. I know Guenter asked that you remove the old driver in a separate patch, but I believe at that point you were also proposing different config option name for the new driver, so you could have removed the old driver only after you added the new one. Since you now use the same configuration option name - which makes perfect sense to me - I think you need to refactor this series. Maybe you could first just move the old driver under drivers/usb/typec/tcpm/ in one patch, and then modify and slit it in another patch. thanks,
On Fri, Nov 05, 2021 at 04:59:53PM +0200, Heikki Krogerus wrote: > On Fri, Nov 05, 2021 at 03:35:57AM +0000, Bryan O'Donoghue wrote: > > Remove the standalone PMIC Type-C driver. We have implemented a full TCPM > > driver which covers and extends the functionality in this driver. > > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > --- > > drivers/usb/typec/Kconfig | 13 -- > > drivers/usb/typec/Makefile | 1 - > > drivers/usb/typec/qcom-pmic-typec.c | 262 ---------------------------- > > 3 files changed, 276 deletions(-) > > delete mode 100644 drivers/usb/typec/qcom-pmic-typec.c > > > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig > > index ab480f38523aa..61fba7bd1671d 100644 > > --- a/drivers/usb/typec/Kconfig > > +++ b/drivers/usb/typec/Kconfig > > @@ -75,19 +75,6 @@ config TYPEC_STUSB160X > > If you choose to build this driver as a dynamically linked module, the > > module will be called stusb160x.ko. > > > > -config TYPEC_QCOM_PMIC > > - tristate "Qualcomm PMIC USB Type-C driver" > > - depends on ARCH_QCOM || COMPILE_TEST > > - depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH > > - help > > - Driver for supporting role switch over the Qualcomm PMIC. This will > > - handle the USB Type-C role and orientation detection reported by the > > - QCOM PMIC if the PMIC has the capability to handle USB Type-C > > - detection. > > - > > - It will also enable the VBUS output to connected devices when a > > - DFP connection is made. > > I don't like that you create point where the support is temporarily > removed for this hardware. I know Guenter asked that you remove the > old driver in a separate patch, but I believe at that point you were > also proposing different config option name for the new driver, so you > could have removed the old driver only after you added the new one. > > Since you now use the same configuration option name - which makes > perfect sense to me - I think you need to refactor this series. Maybe > you could first just move the old driver under drivers/usb/typec/tcpm/ > in one patch, and then modify and slit it in another patch. Or just merge this patch to the next one. thanks,
>> >> I don't like that you create point where the support is temporarily >> removed for this hardware. I know Guenter asked that you remove the >> old driver in a separate patch, but I believe at that point you were >> also proposing different config option name for the new driver, so you >> could have removed the old driver only after you added the new one. >> >> Since you now use the same configuration option name - which makes >> perfect sense to me - I think you need to refactor this series. Maybe >> you could first just move the old driver under drivers/usb/typec/tcpm/ >> in one patch, and then modify and slit it in another patch. No problem with this in principle > Or just merge this patch to the next one. I think this for preference unless Guenter has an objection .. easier/less work --- bod
On Fri, Nov 05, 2021 at 04:05:37PM +0000, Bryan O'Donoghue wrote: > > > > > > > I don't like that you create point where the support is temporarily > > > removed for this hardware. I know Guenter asked that you remove the > > > old driver in a separate patch, but I believe at that point you were > > > also proposing different config option name for the new driver, so you > > > could have removed the old driver only after you added the new one. > > > > > > Since you now use the same configuration option name - which makes > > > perfect sense to me - I think you need to refactor this series. Maybe > > > you could first just move the old driver under drivers/usb/typec/tcpm/ > > > in one patch, and then modify and slit it in another patch. > > No problem with this in principle > > > Or just merge this patch to the next one. > > I think this for preference unless Guenter has an objection .. easier/less > work I understand the logic, so I won't object. Note that I may not have time to review the resulting patch if it ends up changing the same source file to replace one driver with another. Guenter