RE: [PATCH] pinctrl: rzn1: Fix check for used MDIO bus

[Phil Edworthy]

Hi Geert,

On 23 November 2018 09:41 Geert Uytterhoeven wrote:
> Subject: Re: [PATCH] pinctrl: rzn1: Fix check for used MDIO bus
> On Mon, Nov 19, 2018 at 5:18 PM Phil Edworthy wrote:
> > This fixes the check for unused mdio bus setting and the following
> > static checker warning:
> >  drivers/pinctrl/pinctrl-rzn1.c:198 rzn1_pinctrl_mdio_select()
> >  warn: always true condition '(ipctl->mdio_func[mdio] >= 0) => (0-u32max
> >= 0)'
> >
> > It also fixes the return var when calling of_get_child_count()
> 
> I think this should be a separate patch.
Ok, I'll split them.


> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> 
> BTW, I have a question about rzn1_pinctrl_mdio_select():
> 
> static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
>                                      u32 func) {
>         if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
>                 dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
>         ipctl->mdio_func[mdio] = func;
> 
>         dev_dbg(ipctl->dev, "setting mdio%d to %u\n", mdio, func);
> 
>         writel(func, &ipctl->lev2->l2_mdio[mdio]); }
> 
> The check warns the user if it overrides an already initialized MDIO function
> with a different value.
> However, there is no method to uninitialize (reset to -1) mdio_func[], to
> avoid getting the warning.
> 
> For a use case, I was thinking about a DT overlay that would cause the MDIO
> function to be initialized on loading, and needs to uninitialize the MDIO
> function on removing.
> 
> Perhaps that is very unlikely or even impossible, given the function of the
> pins controlled by the MDIO function?
I hadn't considered that DT overlay possibility...
Since this MDIO muxing selects one of several different IP blocks as the
MDIO master, I guess it could happen. However, this is pretty unlikely!

I can't see any way via the pinctrl_ops or pinconf_ops to 'undo' a pin
setting, how would this work?
If a DT overlay causes remove() then probe() to be called again, the driver
resets mdio_func[] in probe(), so it'll work.

Thanks!
Phil