linux-kernel - Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend 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]

Date:   Wed, 30 Jun 2021 09:32:14 +0200
From:   Wolfram Sang <wsa@...nel.org>
To:     Jie Deng <jie.deng@...el.com>
Cc:     linux-i2c@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, mst@...hat.com, jasowang@...hat.com,
        andriy.shevchenko@...ux.intel.com, conghui.chen@...el.com,
        arnd@...db.de, kblaiech@...lanox.com,
        jarkko.nikula@...ux.intel.com, Sergey.Semin@...kalelectronics.ru,
        rppt@...nel.org, loic.poulain@...aro.org, tali.perry1@...il.com,
        u.kleine-koenig@...gutronix.de, bjorn.andersson@...aro.org,
        yu1.wang@...el.com, shuo.a.liu@...el.com, viresh.kumar@...aro.org,
        stefanha@...hat.com, pbonzini@...hat.com
Subject: Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver


> > > +	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.

> > > +	vi->adap.timeout = HZ / 10;
> > Why so short? HZ is the kinda default value.
> 
> 
> Ah... I didn't know the I2C core had already set a default value.
> I will remove this line to use the default one.

Sounds good.

Looking forward to the next version!

Happy hacking,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)