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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <TY1PR01MB1769DE254148618373F2918CF5D40@TY1PR01MB1769.jpnprd01.prod.outlook.com>
Date:   Fri, 23 Nov 2018 10:01:22 +0000
From:   Phil Edworthy <phil.edworthy@...esas.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
CC:     Jacopo Mondi <jacopo@...ndi.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH] pinctrl: rzn1: Fix check for used MDIO bus

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@...cle.com>
> > Signed-off-by: Phil Edworthy <phil.edworthy@...esas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
> 
> 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