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
