On Wed, Nov 29, 2023 at 12:22 AM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Tue, Nov 28, 2023 at 04:42:31PM +0800, Chen-Yu Tsai wrote: > > Some devices are designed and manufactured with some components having > > multiple drop-in replacement options. These components are often > > connected to the mainboard via ribbon cables, having the same signals > > and pin assignments across all options. These may include the display > > panel and touchscreen on laptops and tablets, and the trackpad on > > laptops. Sometimes which component option is used in a particular device > > can be detected by some firmware provided identifier, other times that > > information is not available, and the kernel has to try to probe each > > device. > > > > This change attempts to make the "probe each device" case cleaner. The > > current approach is to have all options added and enabled in the device > > tree. The kernel would then bind each device and run each driver's probe > > function. This works, but has been broken before due to the introduction > > of asynchronous probing, causing multiple instances requesting "shared" > > resources, such as pinmuxes, GPIO pins, interrupt lines, at the same > > time, with only one instance succeeding. Work arounds for these include > > moving the pinmux to the parent I2C controller, using GPIO hogs or > > pinmux settings to keep the GPIO pins in some fixed configuration, and > > requesting the interrupt line very late. Such configurations can be seen > > on the MT8183 Krane Chromebook tablets, and the Qualcomm sc8280xp-based > > Lenovo Thinkpad 13S. > > > > Instead of this delicate dance between drivers and device tree quirks, > > this change introduces a simple I2C component probe. function For a > > given class of devices on the same I2C bus, it will go through all of > > them, doing a simple I2C read transfer and see which one of them responds. > > It will then enable the device that responds. > > > > This requires some minor modifications in the existing device tree. The > > status for all the device nodes for the component options must be set > > to "failed-needs-probe". This makes it clear that some mechanism is > > needed to enable one of them, and also prevents the prober and device > > drivers running at the same time. > > ... > > > +/** > > + * i2c_of_probe_component() - probe for devices of "type" on the same i2c bus > > + * @dev: &struct device of the caller, only used for dev_* printk messages > > + * @type: a string to match the device node name prefix to probe for > > + * > > + * Probe for possible I2C components of the same "type" on the same I2C bus > > + * that have their status marked as "fail". > > Definitely you haven't run kernel-doc validation. Right. Will add missing parts. > > + */ > > ... > > > + return dev_err_probe(dev, -ENODEV, "Could not find %s device node\n", type); > > I haven't noticed clear statement in the description that this API is only for > the ->probe() stages. Will add that to the Context part of the kernel-doc. > ... > > > + if (i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data) < 0) > > + continue; > > This will require the device to be powered on. Are you sure it will be always > the case? This is left as TODO. The devices I have tie the component power supplies to an always on power rail. I guess I could get a trace of the function calls to see if things are running as they should. Not sure if that is enough? ChenYu
