Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver

From: Arnd Bergmann <arnd@arndb.de>
To: Jie Deng <jie.deng@intel.com>
Cc: "Wolfram Sang" <wsa@kernel.org>,
	"Linux I2C" <linux-i2c@vger.kernel.org>,
	virtualization@lists.linux-foundation.org,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	conghui.chen@intel.com, kblaiech@mellanox.com,
	jarkko.nikula@linux.intel.com,
	"Sergey Semin" <Sergey.Semin@baikalelectronics.ru>,
	"Mike Rapoport" <rppt@kernel.org>,
	loic.poulain@linaro.org, "Tali Perry" <tali.perry1@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Bjorn Andersson" <bjorn.andersson@linaro.org>,
	yu1.wang@intel.com, shuo.a.liu@intel.com,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver
Date: Wed, 30 Jun 2021 09:55:49 +0200	[thread overview]
Message-ID: <CAK8P3a0Ew+RS_1buR+1OneH8XEqVjPOr0FGCF5d6CvFQuJqg6g@mail.gmail.com> (raw)
In-Reply-To: <3016ab8b-cbff-1309-6a1f-080703a4130f@intel.com>

On Wed, Jun 30, 2021 at 9:51 AM Jie Deng <jie.deng@intel.com> wrote:
> On 2021/6/30 15:32, Wolfram Sang wrote:
> >>>> +  snprintf(vi->adap.name, sizeof(vi->adap.name), "Virtio I2C Adapter");
> >>> Is there something to add so you can distinguish multiple instances?
> >>> Most people want that.
> >>
> >> I find the I2C core will set a device name "i2c-%d" for this purpose, right?
> >>
> >> I think this name can be used to distinguish the adapter types while
> >> "i2c-%d" can be used to
> >>
> >> distinguish instances. Does it make sense ?
> > That alone does not help. See the 'i2cdetect -l' output of my Renesas
> > board here:
> >
> > i2c-4 i2c             e66d8000.i2c                            I2C adapter
> > i2c-2 i2c             e6510000.i2c                            I2C adapter
> > i2c-7 i2c             e60b0000.i2c                            I2C adapter
> >
> > Notice that the third column carries the base address, so you know which
> > i2c-%d is which physical bus. I don't know if it makes sense in your
> > "virtual" case, but so far it would always print "Virtio I2C Adapter".
> > Maybe it makes sense to add some parent device name, too?
> >
> > And if this is not reasonable, just skip it. As I said, it can be
> > helpful at times, but it is definately not a show stopper.
>
>
> OK. I will add the virtio_device index for this purpose.
> which indicates the unique position on the virtio bus.

Is that position stable across kernel versions? We do have stable naming
for PCI devices and for platform devices that are the parent of a virtio
device, but I would expect the virtio device to be numbered in probe
order instead.

On a related note, we are apparently still missing the bit in the virtio bus
layer that fills in the dev->of_node pointer of the virtio device. Without
this, it is not actually possible to automatically probe i2c devices connected
to a virtio-i2c bus. The same problem came up again with the virtio-gpio
driver that suffers from the same issue.

       Arnd