Messages in this thread | | | Date | Fri, 17 Nov 2023 11:05:33 +0000 | From | Lee Jones <> | Subject | Re: [PATCH v1 0/3] TPS65224 PMIC driver |
| |
On Fri, 10 Nov 2023, Greg KH wrote:
> On Thu, Nov 09, 2023 at 10:22:00AM -0600, Shree Ramamoorthy wrote: > > > I compared 'tps65224.h' with 'tps6594.h', especially register mapping. > > > There are less resources in TPS65224, but I don't see any incompatibility > > > between both PMIC register mappings. Some registers are not used by > > > your TPS65224, and some interrupts are not used either (that's not a > > > problem, they will not trigger, so). Beyond that, I2C and PFSM drivers > > > perform the same things for both PMICs. That's why according to me, > > > nothing prevents from re-using TPS6594 drivers. Even for ADC, which is > > > specific to your TPS65224 indeed, the register range does not overlap > > > with any of TPS6594 registers. You could conditionally add this driver > > > (that's what we did in 'tps6594-core.c' for RTC driver, which is not > > > used > > > for one of the compatibles: you can do something similar for ADC). > > > You will probably add support for others TPS65224 drivers over the next > > > weeks: SPI, ESM, RTC, GPIOs, regulators, watchdog, and ADC. Most of them > > > should be compatible with both TPS6594 and TPS65224, I think (even > > > watchdog driver, which was not developed for TPS6594). ADC will not, > > > but as explained above you can easily deal with this one thanks to > > > the compatible. > > > For 'tps65224-core.c' only, a little bit of work might be necessary to > > > handle your TPS65224 specific functionalities. By using a different DT > > > compatible string, your driver can then select different options (or > > > maybe > > > even different register ranges) for some features based on the > > > compatible. > > > But except for 'tps65xx-core.c', there is "sufficient overlap" to justify > > > sharing as much as possible between TPS65224 and TPS6594, in my > > > opinion. > > > > > > TI is positioning TPS65224 as a separate family from TPS6594, but shared > > software drivers for PMICs that have different use cases would lead to > > confusion. > > Why? No one cares what a driver's name is, only that it works for their > hardware. What different "use case" would cause problems here? > > > Re-scoping the project to accommodate these suggestions would > > negatively affect the timeline we are trying to meet. > > There are no timelines/deadlines with kernel development, sorry, that's > not our issue. > > > We want to include the > > restructure that addresses the compatibility, register maps, and > > functionality similarities, but it would best solved after the upcoming > > deadline has been met. > > Again, no deadline here. Please do the work properly, that's all we > care about. > > > With the growth of PMIC software device drivers, we > > would prefer to have a separate series with the suggested changes and proper > > naming convention to address that while they overlap, the two PMICs devices > > are not a subset. > > Why does the name matter? Again, all that a user cares about is if > their hardware device is supported, the name means nothing here. > > Please do the correct thing and add support for this device to the > existing drivers, that's the correct thing to do. You will save time > and energy and code in the long-run, which is the important thing. > > There is a reason that Linux drivers are, on average, 1/3 smaller than > other operating systems. And that's because they share common code with > other drivers. You aren't allowed to just copy an existing one and add > a few changes and make a whole new driver, you need to modify the > current one. > > thanks, > > greg k-h
Ha! You took the words right out of my mouth!
Thanks.
-- Lee Jones [李琼斯]
| |