linux-kernel - Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI 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: <20240320031253.eutoon7l7nkjehft@vireshk-i7>
Date: Wed, 20 Mar 2024 08:42:53 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Harald Mommer <harald.mommer@...nsynergy.com>
Cc: Haixu Cui <quic_haixcui@...cinc.com>, Mark Brown <broonie@...nel.org>,
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	quic_ztu@...cinc.com, Matti Moell <Matti.Moell@...nsynergy.com>,
	Mikhail Golubev <Mikhail.Golubev@...nsynergy.com>
Subject: Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver.

Hi Harald,

On 19-03-24, 16:05, Harald Mommer wrote:
> On 18.03.24 10:39, Haixu Cui wrote:
> > On 3/4/2024 11:43 PM, Harald Mommer wrote:
> > > +static int virtio_spi_probe(struct virtio_device *vdev)
> > > +{
> > > +    struct device_node *np = vdev->dev.parent->of_node;
> > > +    struct virtio_spi_priv *priv;
> > > +    struct spi_controller *ctrl;
> > > +    int err;
> > > +    u32 bus_num;
> > > +    u16 csi;
> > > +
> > > +    ctrl = devm_spi_alloc_host(&vdev->dev, sizeof(*priv));
> > > +    if (!ctrl)
> > > +        return -ENOMEM;
> > > +
> > > +    priv = spi_controller_get_devdata(ctrl);
> > > +    priv->vdev = vdev;
> > > +    vdev->priv = priv;
> > > +    dev_set_drvdata(&vdev->dev, ctrl);
> > 
> >     ctrl->dev.of_node is not set, so the child node cannot be parsed. I
> > would say, it's necessary to support the virtio spi device node in the
> > format:
> 
> 
> What you most probably want to have here is
> 
>   ctrl->dev.of_node = np;

Looking at how i2c-virtio does it, it should be tied to the device itself
instead of its parent:

ctrl->dev.of_node = vdev->dev.of_node;

> >     virtio-spi@...13000 {
> >         compatible = "virtio,mmio";
> >         reg = <0x4b013000 0x1000>;
> >         interrupts = <0 497 4>;
> > 
> >         spi {
> >             compatible = "virtio,device2d";
> >             #address-cells = <1>;
> >             #size-cells = <0>;
> > 
> >             spidev {
> >                 compatible = "xxx";
> >                 reg = <0>;
> >                 spi-max-frequency = <100000>;
> >             };
> >         };
> >     };

Right, some work was done in the past to standardize these compatibles:

$ git log -p --stat --reverse 0d8c9e7d4b40..694a1116b405

-- 
viresh