Re: [PATCH 1/3] regulator: max14577: Add proper dt-compatible strings

[Krzysztof Kozlowski <krzk@kernel.org>]

On Fri, Mar 06, 2020 at 02:51:22PM +0100, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 24.02.2020 21:12, Krzysztof Kozlowski wrote:
> > On Mon, Feb 24, 2020 at 03:08:05PM +0100, Marek Szyprowski wrote:
> >> On 21.02.2020 18:13, Mark Brown wrote:
> >>> On Fri, Feb 21, 2020 at 02:23:57PM +0100, Marek Szyprowski wrote:
> >>>> On 21.02.2020 13:38, Mark Brown wrote:
> >>>>> We could just remove the compatible strings from the binding
> >>>>> documentation, they won't do any harm if we don't use them.
> >>>> Frankly I have no strong opinion on this. I've just wanted to fix the
> >>>> broken autoloading of the drivers compiled as modules.
> >>> Shouldn't adding the relevant module table for the platform devices work
> >>> just as well for that?  Possibly also deleting the of_compatible bits in
> >>> the MFD as well, ISTR that's needed to make the platform device work.
> >> Right. This will work too. MFD cells will match to their drivers by the
> >> name and modalias strings will be correct. The question is which
> >> approach is preffered? Krzysztof? I've checked other mfd drivers, but I
> >> cannot find any pattern in this area.
> > I would guess that adding MODULE_DEVICE_TABLE() for OF-matches in main
> > MFD driver would fix the issue... otherwise the same problem we have
> > with max77693 (also MUIC/extcon/regulator/charger).
> 
> Indeed, there is a same problem with max77963:
> 
> max77963-muic driver lacks compatible and has wrong platform modalias 
> ("extcon-max77963"),
> 
> max77963-charger driver lacks compatible,
> 
> max77963-haptic driver lacks compatible.
> 
> > Some of these drivers (I guess only charger) bind to a OF node so they
> > need a compatible. I think we added this to regulators and extcon for
> > symmetry.
> > Without this binding, the charger would need to read a specific child
> > node from parent. This make them tightly coupled. It seems to me more
> > robust for each component to bind to his own node, when needed.
> 
> Extcon would also need its node when support for it will be added to 
> dwc2 driver. Having compatible strings in the nodes simplifies matching 
> and makes it almost automatic.
> 
> > Another reason of adding compatibles was an idea of reusability of
> > MFD children (between different MFD drivers or even standalone) but it
> > never got implemented (children still depend on parent significantly).
> 
> So far, there is no such case.
> 
> > In general, I like the approach of children with compatibles but I will
> > not argue against changing the drivers. They could really use some
> > cleanup :)
> > Long time I tried to remove the support for platform_data [1] - maybe
> > let's continue?
> >
> > [1] https://lore.kernel.org/lkml/20170217200200.4521-1-krzk@kernel.org/
> 
> Cleanup of the driver is another story, completely independent of fixing 
> this issue imho.
> 
> krzk: could you then specify if you are against or after the proposed 
> changes?

Since we already have compatibles and some of the children require them
(charger), I vote in favor of this patch and for keeping compatibles.

Best regards,
Krzysztof