linux-kernel - Re: [PATCH v2 1/2] bus: imx-weim: support multiple address ranges per child node

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]

Date:   Wed, 5 Dec 2018 10:08:16 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Shawn Guo <shawnguo@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>, Rob Herring <robh@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] bus: imx-weim: support multiple address ranges per
 child node

Hello Shawn, many thanks for the patch review, I really appreciate it !

On Wed, Dec 5, 2018 at 2:52 AM Shawn Guo <shawnguo@...nel.org> wrote:
>
> On Fri, Nov 30, 2018 at 03:56:23PM -0500, thesven73@...il.com wrote:
> > From: Sven Van Asbroeck <TheSven73@...glemail.com>
> >
> > Ensure that timing values for the child node are applied to
> > all chip selects in the child's address ranges.
> >
>
> I'm not sure about that.  Shouldn't we have another child node for
> different chip select, something like below?
>
> &weim {
>         acme@0,0 {
>                 compatible = "acme,whatever";
>                 reg = <0 0 0x100>, <0 0x400000 0x800>;
>                 fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
>                                       0x00000000 0xa0000240 0x00000000>;
>         };
>
>         acme@1,400000 {
>                 compatible = "acme,whatever";
>                 reg = <1 0x400000 0x800>;
>                 fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
>                                       0x00000000 0xa0000240 0x00000000>;
>         };
>
> Shawn

I am submitting patches for a device that spans chip selects :(
And such a device needs multiple address changes with different chip selects.

Imagine we have an acme device, which contains a control and a fifo region,
on different chip selects:

&weim {
        acme@0 {
                compatible = "acme";
                reg = <0 0x0 0x100>, <1 0x0 0x100>;
        };
};

Now in probe we can access both regions:
int acme_probe(struct platform_device *pdev)
{
        control_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        fifo_res =    platform_get_resource(pdev, IORESOURCE_MEM, 1);
        /* all ok */
}

But, if we have two separate child nodes, we also get two calls to probe(),
which assumes two devices on the bus, and that is incorrect:

&weim {
        acme@0 {
                compatible = "acme";
                reg = <0 0x0 0x100>;
        };
        acme@1 {
                compatible = "acme";
                reg = <1 0x0 0x100>;
        };
};

int acme_probe(struct platform_device *pdev)
{
        control_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        /* next call always fails */
        fifo_res =    platform_get_resource(pdev, IORESOURCE_MEM, 1);
}

For my patchset, Rob Herring suggested I made changes to the imx-weim driver
to accommodate multi-chipselect devices.

See the conversation below between Rob Herring and myself:
https://lkml.org/lkml/2018/11/30/390

If you are not entirely satisfied with my patch, then perhaps you
could think of another way to support multi-chipselect devices?

Many thanks,
Sven