Received-SPF: pass (google.com: domain of linux-kernel+bounces-108404-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 20 Mar 2024 08:42:53 +0530
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Harald Mommer <harald.mommer@opensynergy.com>
Cc: Haixu Cui <quic_haixcui@quicinc.com>, Mark Brown <broonie@kernel.org>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	quic_ztu@quicinc.com, Matti Moell <Matti.Moell@opensynergy.com>,
	Mikhail Golubev <Mikhail.Golubev@opensynergy.com>
Subject: Re: [PATCH v2 3/3] virtio-spi: Add virtio SPI driver.
Message-ID: <20240320031253.eutoon7l7nkjehft@vireshk-i7>
References: <20240304154342.44021-1-Harald.Mommer@opensynergy.com>
 <20240304154342.44021-4-Harald.Mommer@opensynergy.com>
 <99afc631-c02b-42da-a8d4-87c65087f390@quicinc.com>
 <5dedcd26-ed59-415f-b978-87a546a0816d@opensynergy.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5dedcd26-ed59-415f-b978-87a546a0816d@opensynergy.com>

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@4b013000 {
> > ??????? 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