| Message ID | 20220203164629.1711958-1-vladimir.zapolskiy@linaro.org |
|---|---|
| Headers | show |
| Series | i2c: qcom-cci: fixes and updates | expand |
Hi, > The new bus adapter specific bus_regulator from commit 5a7b95fb993e > ("i2c: core: support bus regulator controlling in adapter") is reused, Reusing is nice, of course, but I hope you noticed that I needed to revert this feature: a19f75de73c2 ("Revert "i2c: core: support bus regulator controlling in adapter"") The thread to get it re-applied is currently here: https://lore.kernel.org/all/20220106122452.18719-1-wsa@kernel.org/ Happy hacking, Wolfram
Hi Wolfram, On 2/11/22 7:49 PM, Wolfram Sang wrote: > Hi, > >> The new bus adapter specific bus_regulator from commit 5a7b95fb993e >> ("i2c: core: support bus regulator controlling in adapter") is reused, > > Reusing is nice, of course, but I hope you noticed that I needed to > revert this feature: > > a19f75de73c2 ("Revert "i2c: core: support bus regulator controlling in adapter"") yes, I've seen it, and as far as I understand it's expected to get it back after the regression fixes. > The thread to get it re-applied is currently here: > > https://lore.kernel.org/all/20220106122452.18719-1-wsa@kernel.org/ > A presented change in the series is I2C controller specific, so it works well on top of the reverted feature, however it has a potential to be simplified after the re-application. Wolfram, can you please share your opinion on device tree binding name and placement for an SDA/SDC pull-up controlled by a regulator? See https://lore.kernel.org/all/682b7ffe-e162-bcf7-3c07-36b3a39c25ab@linaro.org Thank you in advance. -- Best wishes, Vladimir
> > Reusing is nice, of course, but I hope you noticed that I needed to > > revert this feature: > > > > a19f75de73c2 ("Revert "i2c: core: support bus regulator controlling in adapter"") > > yes, I've seen it, and as far as I understand it's expected to get it > back after the regression fixes. True, but work on this has stalled, sadly. I am gathering interested parties for the topic here :) > Wolfram, can you please share your opinion on device tree binding name and > placement for an SDA/SDC pull-up controlled by a regulator? For efficiency reasons, not before bus regulator has been applied again because the above question depends on it IIUC. Until then, I'll work on other items of my too long todo list.
> Vladimir Zapolskiy (9): > dt-bindings: i2c: qcom-cci: add QCOM SM8450 compatible > dt-bindings: i2c: qcom-cci: add description of a vbus-supply property > i2c: qcom-cci: don't delete an unregistered adapter > i2c: qcom-cci: don't put a device tree node before i2c_add_adapter() > i2c: qcom-cci: initialize CCI controller after registration of adapters > i2c: qcom-cci: simplify probe by removing one loop over busses > i2c: qcom-cci: simplify access to bus data structure > i2c: qcom-cci: add support of optional vbus-supply regulators > i2c: qcom-cci: add sm8450 compatible Patches 3+4 are already upstream. I wonder if patches 1+9 could be applied to for-next also? Or is the vbus-supply a hard dependency here? Patches 6+7 could probably also be resent individually after some rebasing.
Hi Wolfram, On 2/17/22 9:57 PM, Wolfram Sang wrote: > >> Vladimir Zapolskiy (9): >> dt-bindings: i2c: qcom-cci: add QCOM SM8450 compatible >> dt-bindings: i2c: qcom-cci: add description of a vbus-supply property >> i2c: qcom-cci: don't delete an unregistered adapter >> i2c: qcom-cci: don't put a device tree node before i2c_add_adapter() >> i2c: qcom-cci: initialize CCI controller after registration of adapters >> i2c: qcom-cci: simplify probe by removing one loop over busses >> i2c: qcom-cci: simplify access to bus data structure >> i2c: qcom-cci: add support of optional vbus-supply regulators >> i2c: qcom-cci: add sm8450 compatible > > Patches 3+4 are already upstream. I wonder if patches 1+9 could be > applied to for-next also? Or is the vbus-supply a hard dependency here? > Patches 6+7 could probably also be resent individually after some > rebasing. > thank you for applying the fixes, 1/9 and 9/9 are also good to be applied for-next, there is no dependency on vbus-supply, so I would appreciate, if you take two more changes. As you suggested I'd start working on a generic support of such an optional bus supplier property, I believe at the moment everything is quite clear, I'll start from testing the previous solution, however my preference is to connect regulator on/off to master controller pm ops rather than slave pm ops. Additionally I am not quite satisfied with 'vbus-supply' name, some name without a confusing starting 'v' should be preferred IMO, like 'i2c-bus-supply' or 'sda-scl-supply', name suggestions are welcome. -- Best wishes, Vladimir
> thank you for applying the fixes, 1/9 and 9/9 are also good to be applied > for-next, there is no dependency on vbus-supply, so I would appreciate, if > you take two more changes. Done now. > As you suggested I'd start working on a generic support of such an optional > bus supplier property, I believe at the moment everything is quite clear, > I'll start from testing the previous solution, however my preference is > to connect regulator on/off to master controller pm ops rather than slave > pm ops. Additionally I am not quite satisfied with 'vbus-supply' name, Did I get this right? You want to reimplement bus regulator handling in the bus driver when we already have pending patches to add it to the I2C core?
The main intention of the patch series is to add support of vbus regulators, which are commonly connected to CCI I2C busses. The new bus adapter specific bus_regulator from commit 5a7b95fb993e ("i2c: core: support bus regulator controlling in adapter") is reused, however its control is connected to runtime pm of the I2C master controller rather than runtime pm of slaves. In addition the series adds new compatible value for CCI found on QCOM SM8450 SoC. Vladimir Zapolskiy (9): dt-bindings: i2c: qcom-cci: add QCOM SM8450 compatible dt-bindings: i2c: qcom-cci: add description of a vbus-supply property i2c: qcom-cci: don't delete an unregistered adapter i2c: qcom-cci: don't put a device tree node before i2c_add_adapter() i2c: qcom-cci: initialize CCI controller after registration of adapters i2c: qcom-cci: simplify probe by removing one loop over busses i2c: qcom-cci: simplify access to bus data structure i2c: qcom-cci: add support of optional vbus-supply regulators i2c: qcom-cci: add sm8450 compatible .../devicetree/bindings/i2c/i2c-qcom-cci.txt | 9 +- drivers/i2c/busses/i2c-qcom-cci.c | 159 ++++++++++++------ 2 files changed, 114 insertions(+), 54 deletions(-)