linux-kernel - Re: RE: [PATCH v18 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver

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: <20201006213143.GD701433@xaphan>
Date:   Tue, 6 Oct 2020 16:31:43 -0500
From:   Michael Auchter <michael.auchter@...com>
To:     Ben Levinsky <BLEVINSK@...inx.com>
Cc:     "Ed T. Mooring" <emooring@...inx.com>,
        "sunnyliangjy@...il.com" <sunnyliangjy@...il.com>,
        "punit1.agrawal@...hiba.co.jp" <punit1.agrawal@...hiba.co.jp>,
        Stefano Stabellini <stefanos@...inx.com>,
        Michal Simek <michals@...inx.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "mathieu.poirier@...aro.org" <mathieu.poirier@...aro.org>,
        "linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: RE: [PATCH v18 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver

On Tue, Oct 06, 2020 at 07:15:49PM +0000, Ben Levinsky wrote:
> 
> Hi Michael,
> 
> Thanks for the review
> 

< ... snip ... >

> > > +	z_rproc = rproc->priv;
> > > +	z_rproc->dev.release = zynqmp_r5_release;
> > 
> > This is the only field of z_rproc->dev that's actually initialized, and
> > this device is not registered with the core at all, so zynqmp_r5_release
> > will never be called.
> > 
> > Since it doesn't look like there's a need to create this additional
> > device, I'd suggest:
> > 	- Dropping the struct device from struct zynqmp_r5_rproc
> > 	- Performing the necessary cleanup in the driver remove
> > 	  callback instead of trying to tie it to device release
> 
> For the most part I agree. I believe the device is still needed for
> the mailbox client setup.
> 
> As the call to mbox_request_channel_byname() requires its own device
> that has the corresponding child node with the corresponding
> mbox-related properties.
> 
> With that in mind, is it still ok to keep the device node?

Ah, I see. Thanks for the clarification!

Instead of manually dealing with the device node creation for the
individual processors, perhaps it makes more sense to use
devm_of_platform_populate() to create them. This is also consistent with
the way the TI K3 R5F remoteproc driver does things.

Cheers,
 Michael