Messages in this thread | | | Subject | Re: [PATCH v8] i2c: virtio: add a virtio i2c frontend driver | From | Jie Deng <> | Date | Fri, 19 Mar 2021 13:31:34 +0800 |
| |
On 2021/3/19 11:54, Viresh Kumar wrote: > On 18-03-21, 15:52, Arnd Bergmann wrote: >> Allowing multiple virtio-i2c controllers in one system, and multiple i2c >> devices attached to each controller is clearly something that has to work. > Good. > >> I don't actually see a limitation though. Viresh, what is the problem >> you see for having multiple controllers? > I thought this would be a problem in that case as we are using the global > virtio_adapter here. > > + vi->adap = &virtio_adapter; > + i2c_set_adapdata(vi->adap, vi); > > Multiple calls to probe() will end up updating the same pointer inside adap. > > + vi->adap->dev.parent = &vdev->dev; > > Same here, overwrite. > > + /* Setup ACPI node for controlled devices which will be probed through ACPI */ > + ACPI_COMPANION_SET(&vi->adap->dev, ACPI_COMPANION(pdev)); > + vi->adap->timeout = HZ / 10; > > These may be fine, but still not ideal I believe. > > + ret = i2c_add_adapter(vi->adap); > i > This should be a problem as well, we must be adding this to some sort of list, > doing some RPM stuff, etc ? > > Jie, the solution is to allocate memory for adap at runtime in probe and remove > the virtio_adapter structure completely.
If you want to support that. Then I think we don't need to change the following at all.
> + .algo = &virtio_algorithm, > + > + return ret; > + > + vi->adap = virtio_adapter; This is strange, why are you allocating memory for adapter twice ? Once for virtio_adapter and once for vi->adap ? Either fill the fields directly for v->adap here and remove virtio_adapter or make vi->adap a pointer.
| |