| Message ID | 20170519173412.68053-2-cbostic@linux.vnet.ibm.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sat, May 20, 2017 at 3:34 AM, Christopher Bostic <cbostic@linux.vnet.ibm.com> wrote: > Add IR35221 to i2c4 and i2c5 buses. This is similar to the patch that I sent on the 2nd. I took your patch as it adds the device at 0x71 which I did not know about. Cheers, Joel > > Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > --- > V2 - Convert I2C bus addresses to 7 bit right aligned. > - Add IR35221 to device tree documentation. > --- > .../devicetree/bindings/i2c/trivial-devices.txt | 1 + > arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 20 ++++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > index cdd7b48..d504271 100644 > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt > @@ -53,6 +53,7 @@ fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec > gmt,g751 G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface > infineon,slb9635tt Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz) > infineon,slb9645tt Infineon SLB9645 I2C TPM (new protocol, max 400khz) > +infineon,ir35221 Infineon IR35221 Digital DC-DC Multiphase Converter > isil,isl29028 Intersil ISL29028 Ambient Light and Proximity Sensor > maxim,ds1050 5 Bit Programmable, Pulse-Width Modulator > maxim,max1237 Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts > index 5d83a76..b0338db 100644 > --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts > @@ -394,6 +394,16 @@ > compatible = "ti,tmp423"; > reg = <0x4c>; > }; > + > + ir35221@70 { > + compatible = "infineon,ir35221"; > + reg = <0x70>; > + }; > + > + ir35221@71 { > + compatible = "infineon,ir35221"; > + reg = <0x71>; > + }; > }; > > > @@ -408,6 +418,16 @@ > compatible = "ti,tmp423"; > reg = <0x4c>; > }; > + > + ir35221@70 { > + compatible = "infineon,ir35221"; > + reg = <0x70>; > + }; > + > + ir35221@71 { > + compatible = "infineon,ir35221"; > + reg = <0x71>; > + }; > }; > > &i2c6 { > -- > 1.8.2.2 >
On Wed, May 24, 2017 at 10:58 AM, Joel Stanley <joel@jms.id.au> wrote: > On Sat, May 20, 2017 at 3:34 AM, Christopher Bostic > <cbostic@linux.vnet.ibm.com> wrote: >> Add IR35221 to i2c4 and i2c5 buses. > > This is similar to the patch that I sent on the 2nd. > > I took your patch as it adds the device at 0x71 which I did not know about. > This breaks userspace, as userspace assumes it can access the i2c devices through /dev/i2c-x. Do you have a ticket opened for fixing userspace? Cheers, Joel
> On May 24, 2017, at 2:20 AM, Joel Stanley <joel@jms.id.au> wrote: > > On Wed, May 24, 2017 at 10:58 AM, Joel Stanley <joel@jms.id.au> wrote: >> On Sat, May 20, 2017 at 3:34 AM, Christopher Bostic >> <cbostic@linux.vnet.ibm.com> wrote: >>> Add IR35221 to i2c4 and i2c5 buses. >> >> This is similar to the patch that I sent on the 2nd. >> >> I took your patch as it adds the device at 0x71 which I did not know about. >> > > This breaks userspace, as userspace assumes it can access the i2c > devices through /dev/i2c-x. > Joel, or anyone able to answer… Do you know which application? I wasn’t aware of any applications using i2c-dev... Can we simply remove i2c-dev from the config? thx - brad > Do you have a ticket opened for fixing userspace? > > Cheers, > > Joel
On Thu, May 25, 2017 at 4:32 AM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: > >> On May 24, 2017, at 2:20 AM, Joel Stanley <joel@jms.id.au> wrote: >> >> On Wed, May 24, 2017 at 10:58 AM, Joel Stanley <joel@jms.id.au> wrote: >>> On Sat, May 20, 2017 at 3:34 AM, Christopher Bostic >>> <cbostic@linux.vnet.ibm.com> wrote: >>>> Add IR35221 to i2c4 and i2c5 buses. >>> >>> This is similar to the patch that I sent on the 2nd. >>> >>> I took your patch as it adds the device at 0x71 which I did not know about. >>> >> >> This breaks userspace, as userspace assumes it can access the i2c >> devices through /dev/i2c-x. >> > > Joel, or anyone able to answer… > > Do you know which application? I wasn’t aware of any applications using i2c-dev... The one I noticed was vrm-control.sh. > Can we simply remove i2c-dev from the config? We could, but that would break other bits of the p9 userspace. Taking a look a the root filesystem, we have the following applications that use the /dev/i2c interfaces: $ grep -rl i2cset * usr/sbin/i2cset usr/bin/avsbus-disable.sh usr/bin/avsbus-workaround.sh usr/bin/avsbus-enable.sh usr/bin/vrm-control.sh usr/bin/vcs_on.sh usr/bin/ucd_disable_vcs.sh usr/bin/vcs_off.sh grep -rl i2cget * usr/sbin/i2cget usr/bin/vrm-control.sh Cheers, Joel
> On May 24, 2017, at 9:16 PM, Joel Stanley <joel@jms.id.au> wrote: > > On Thu, May 25, 2017 at 4:32 AM, Brad Bishop > <bradleyb@fuzziesquirrel.com> wrote: >> >>> On May 24, 2017, at 2:20 AM, Joel Stanley <joel@jms.id.au> wrote: >>> >>> On Wed, May 24, 2017 at 10:58 AM, Joel Stanley <joel@jms.id.au> wrote: >>>> On Sat, May 20, 2017 at 3:34 AM, Christopher Bostic >>>> <cbostic@linux.vnet.ibm.com> wrote: >>>>> Add IR35221 to i2c4 and i2c5 buses. >>>> >>>> This is similar to the patch that I sent on the 2nd. >>>> >>>> I took your patch as it adds the device at 0x71 which I did not know about. >>>> >>> >>> This breaks userspace, as userspace assumes it can access the i2c >>> devices through /dev/i2c-x. >>> >> >> Joel, or anyone able to answer… >> >> Do you know which application? I wasn’t aware of any applications using i2c-dev... > > The one I noticed was vrm-control.sh. > >> Can we simply remove i2c-dev from the config? > > We could, but that would break other bits of the p9 userspace. > > Taking a look a the root filesystem, we have the following > applications that use the /dev/i2c interfaces: > > $ grep -rl i2cset * > usr/sbin/i2cset > usr/bin/avsbus-disable.sh > usr/bin/avsbus-workaround.sh > usr/bin/avsbus-enable.sh > usr/bin/vrm-control.sh > usr/bin/vcs_on.sh > usr/bin/ucd_disable_vcs.sh > usr/bin/vcs_off.sh > Thanks Joel. I had forgotten about these. I’m not sure how to handle this. We obviously still need to run these workarounds, but we also need the regulator temperatures. The only thing I can think of is exposing what these scripts need via sysfs attributes. Is there any alternative? > grep -rl i2cget * > usr/sbin/i2cget > usr/bin/vrm-control.sh > > Cheers, > > Joel
On Thu, May 25, 2017 at 10:54 AM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: >>>> This breaks userspace, as userspace assumes it can access the i2c >>>> devices through /dev/i2c-x. >>>> >>> >>> Joel, or anyone able to answer… >>> >>> Do you know which application? I wasn’t aware of any applications using i2c-dev... >> >> The one I noticed was vrm-control.sh. > I’m not sure how to handle this. We obviously still need to run these workarounds, > but we also need the regulator temperatures. > > The only thing I can think of is exposing what these scripts need via sysfs attributes. > > Is there any alternative? There are two options: Write from userspace We could unbind the driver, make the relevant configuration changes in userspace, and then re-bind the driver. This has the benefit of being sure that the driver will still work, and does not require any kernel work. A sub-option of this is to do the same without unbinding the driver. Userspace can specify a "force" flag in the ioctl, which allows it to write to the i2c device even when a kernel driver is bound. The upside here is less messing around with unbinding, but the drawback is userspace may change the hardware state that the driver depends on, leading to instability. We could audit the drivers to make sure the state we're changing from userspace does not affect the driver, and/or fix the driver so it still behaves. Add functionality to the driver Look at adding the required configuration to the driver. Either through device tree properties, or sysfs files. This is would require looking at the writes we need to do, and working out how they can be integrated. As discussed today, we will go with writing from userspace after unbinding the driver. As I've already merged the ir35221 patch into the kernel, completing the unbinding work is blocking an update of the kernel. Do we have a ticket opened for this work? Cheers, Joel
diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt index cdd7b48..d504271 100644 --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt @@ -53,6 +53,7 @@ fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec gmt,g751 G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface infineon,slb9635tt Infineon SLB9635 (Soft-) I2C TPM (old protocol, max 100khz) infineon,slb9645tt Infineon SLB9645 I2C TPM (new protocol, max 400khz) +infineon,ir35221 Infineon IR35221 Digital DC-DC Multiphase Converter isil,isl29028 Intersil ISL29028 Ambient Light and Proximity Sensor maxim,ds1050 5 Bit Programmable, Pulse-Width Modulator maxim,max1237 Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts index 5d83a76..b0338db 100644 --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts @@ -394,6 +394,16 @@ compatible = "ti,tmp423"; reg = <0x4c>; }; + + ir35221@70 { + compatible = "infineon,ir35221"; + reg = <0x70>; + }; + + ir35221@71 { + compatible = "infineon,ir35221"; + reg = <0x71>; + }; }; @@ -408,6 +418,16 @@ compatible = "ti,tmp423"; reg = <0x4c>; }; + + ir35221@70 { + compatible = "infineon,ir35221"; + reg = <0x70>; + }; + + ir35221@71 { + compatible = "infineon,ir35221"; + reg = <0x71>; + }; }; &i2c6 {
Add IR35221 to i2c4 and i2c5 buses. Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> --- V2 - Convert I2C bus addresses to 7 bit right aligned. - Add IR35221 to device tree documentation. --- .../devicetree/bindings/i2c/trivial-devices.txt | 1 + arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+)