Re: [PATCH] mux: mmio: use reg property when parent device is not a syscon

[Andrew Davis]

On 5/16/23 1:44 PM, Krzysztof Kozlowski wrote:
> On 16/05/2023 20:37, Krzysztof Kozlowski wrote:
> > On 15/05/2023 21:19, Andrew Davis wrote:
> > > The DT binding for the reg-mux compatible states it can be used when the
> > > "parent device of mux controller is not syscon device". It also allows
> > > for a reg property. When the parent device is indeed not a syscon device,
> > > nor is it a regmap provider, we should fallback to using that reg
> > > property to identify the address space to use for this mux.
> > >
> > > Signed-off-by: Andrew Davis <afd@xxxxxx>
> > > ---
> > >   drivers/mux/mmio.c | 9 ++++++---
> > >   1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c
> > > index 44a7a0e885b8..42e00b9fd0a9 100644
> > > --- a/drivers/mux/mmio.c
> > > +++ b/drivers/mux/mmio.c
> > > @@ -44,10 +44,13 @@ static int mux_mmio_probe(struct platform_device *pdev)
> > >   	int ret;
> > >   	int i;
> > >   
> > > -	if (of_device_is_compatible(np, "mmio-mux"))
> > > +	if (of_device_is_compatible(np, "mmio-mux")) {
> > >   		regmap = syscon_node_to_regmap(np->parent);
> > > -	else
> > > -		regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV);
> > > +	} else {
> > > +		regmap = dev_get_regmap(dev->parent, NULL);
> > > +		if (!regmap)
> > > +			regmap = device_node_to_regmap(np) ?: ERR_PTR(-ENODEV);
> >
> > Unless I miss something obvious, the original code is simply bogus and
> > wrong. I would like to give here Rb tag... but maybe I miss something
> > obvious. Why mux cannot be a device with MMIO itself? Binding allows it
> > which would be perfectly proper description of hardware.
>
> OK, I see now original binding and it did not allow 'reg' property, thus
> binding was matching driver. Commit 9b358af7c818 ("dt-bindings: mux:
> Convert mux controller bindings to schema") introduced reg, which is
> reasonable.
>
> The driver change is reasonable as well - we should not need syscon
> parent... however, I think the code should be different. If reg is
> present you should use it first. If reg is missing, use parent. This
> solves the case when node with reg will be put inside something else
> which has regmap, but should not be used for reg-mux.

Makes sense to me. I didn't do that to begin with to keep the
functionality the same, but I don't see any users of this with both
"reg" and a parent with regmap today. Will flip the logic for v2.

Thanks,
Andrew

> Best regards,
> Krzysztof