Re: [PATCH v5 04/11] pwm: max7360: Add MAX7360 PWM support

[Andy Shevchenko]

On Thu, Mar 27, 2025 at 03:28:08PM +0100, Mathieu Dubois-Briand wrote:
> On Wed Mar 26, 2025 at 4:49 PM CET, Andy Shevchenko wrote:

> > The use of this API is inappropriate here AFAICT. It drops the parent refcount
> > and on the second call to it you will have a warning from refcount library.
> >
> > It should be as simple as device_set_node().
> >
> > >         }
> >
> > With that, the conditional becomes
> >
> > 	} else if (is_of_node(fwnode)) {
> > 		device_set_node(&pdev->dev, fwnode);
> > 	}
> >
> > where fwnode is something like
> >
> > 	struct fwnode_handle *fwnode = dev_fwnode(parent);
> 
> I tried to use device_set_node(), but then I got some other issue: as we
> now have several devices with the same firmware node, they all share the
> same properties. In particular, if we do use pinctrl- properties to
> apply some pinmmuxing, all devices will try to apply this pinmuxing and
> of course all but one will fail.
> 
> And this makes me think again about the whole thing, maybe copying the
> fwnode or of_node from the parent is not the way to go.
> 
> So today we rely on the parent node for four drivers:
> - keypad and rotary, just to ease a bit the parsing of some properties,
>   such as the keymap with matrix_keypad_build_keymap(). I can easily do
>   it another way.
> - PWM and pinctrl drivers, are a bit more complicated, as in both case
>   the device tree node associated with the device is used internally. In
>   one case to find the correct PWM device for PWM clients listed in the
>   device tree, in the other case to find the pinctrl device when
>   applying pinctrl described in the device tree.
> 
> So maybe I have to find a better way for have this association. One way
> would be to modify the device tree bindings to add a PWM and a pinctrl
> node, with their own compatible, so they are associated to the
> corresponding device. But maybe there is a better way to do it.

Okay, so the main question now, why do the device share their properties
to begin with? It can be done via fwnode graph or similar APIs (in case
it is _really_ needed).

-- 
With Best Regards,
Andy Shevchenko