linux-kernel - Re: [PATCH] ARM: dts: NSP: Use hardware I2C for BCM958625HR

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]

Message-ID: <c11853ec-0a0b-4621-c0cb-e366d36c9592@gmail.com>
Date:   Fri, 13 Dec 2019 14:47:31 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Ray Jui <ray.jui@...adcom.com>,
        linux-arm-kernel@...ts.infradead.org
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        "maintainer:BROADCOM IPROC ARM ARCHITECTURE" 
        <bcm-kernel-feedback-list@...adcom.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: dts: NSP: Use hardware I2C for BCM958625HR



On 12/13/2019 2:11 PM, Ray Jui wrote:
> 
> 
> On 2019-12-13 11:51 a.m., Florian Fainelli wrote:
>> Now that the i2c-bcm-iproc driver has been fixed to permit reading more
>> than 63 bytes in a single transaction with commit fd01eecdf959 ("i2c:
>> iproc: Fix i2c master read more than 63 bytes") we no longer need to
>> bitbang i2c over GPIOs which was necessary before to allow the
>> PHYLINK/SFP subsystems to read SFP modules.
>>
> 
> This is good to hear!
> 
>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>> ---
>>   arch/arm/boot/dts/bcm958625hr.dts | 15 +++++----------
>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm958625hr.dts
>> b/arch/arm/boot/dts/bcm958625hr.dts
>> index a2c9de35ddfb..536fb24f38bb 100644
>> --- a/arch/arm/boot/dts/bcm958625hr.dts
>> +++ b/arch/arm/boot/dts/bcm958625hr.dts
>> @@ -55,18 +55,9 @@
>>           priority = <200>;
>>       };
>>   -    /* Hardware I2C block cannot do more than 63 bytes per transfer,
>> -     * which would prevent reading from a SFP's EEPROM (256 byte).
>> -     */
>> -    i2c1: i2c {
>> -        compatible = "i2c-gpio";
>> -        sda-gpios = <&gpioa 5 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> -        scl-gpios = <&gpioa 4 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> -    };
>> -
> 
> So I suppose GPIO 4 and 5 from the 'gpioa' controller are tied to the
> same SCL/SDA pins from i2c0 and they are internally muxed, right?

Correct.

> 
> Is the mux to GPIO done automatically when pins are configured as GPIO,
> and therefore you don't require any additional changes to pinmux to make
> this work, after changing it back to use real I2C0 block below?

Yes indeed.

> 
>>       sfp: sfp {
>>           compatible = "sff,sfp";
>> -        i2c-bus = <&i2c1>;
>> +        i2c-bus = <&i2c0>;
>>           mod-def0-gpios = <&gpioa 28 GPIO_ACTIVE_LOW>;
>>           los-gpios = <&gpioa 24 GPIO_ACTIVE_HIGH>;
>>           tx-fault-gpios = <&gpioa 30 GPIO_ACTIVE_HIGH>;
>> @@ -74,6 +65,10 @@
>>       };
>>   };
>>   +&i2c0 {
>> +    status = "okay";
>> +};
>> +
>>   &amac0 {
>>       status = "okay";
>>   };
>>
> 
> Change looks good to me.
> 
> Reviewed-by: Ray Jui <ray.jui@...adcom.com>

Thanks!
-- 
Florian