On 15/11/2023 04:57, Chris Packham wrote:
> Add bus-reset-gpios and bus-reset-duration-us properties to the binding
> description for i2c busses. These can be used to describe hardware where
> a common reset GPIO is connected to all downstream devices on and I2C
> bus. This reset will be asserted then released before the downstream
> devices on the bus are probed.
>
> Signed-off-by: Chris Packham <[email protected]>
> ---
>
...
>
> Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index fc3dd7ec0445..3f95d71b9985 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -99,6 +99,14 @@ wants to support one of the below features, it should adapt these bindings.
> indicates that the system is accessible via this bus as an endpoint for
> MCTP over I2C transport.
>
> +- bus-reset-gpios:
> + GPIO pin providing a common reset for all downstream devices. This GPIO
> + will be asserted then released before the downstream devices are probed.
I initially reviewed it, but did not think enough about it. After more
consideration, I believe this is not a property of the I2C bus
controller. This is a property of each device, even if the GPIO is the same.
Linux kernel already supports shared GPIO, so you only need
enable-ref-counting on it.
Putting it into the controller bindings looks like solving OS issue with
incorrect hardware description.
Best regards,
Krzysztof
Hi Krystof,
On 16/11/23 10:29, Krzysztof Kozlowski wrote:
> On 15/11/2023 04:57, Chris Packham wrote:
>> Add bus-reset-gpios and bus-reset-duration-us properties to the binding
>> description for i2c busses. These can be used to describe hardware where
>> a common reset GPIO is connected to all downstream devices on and I2C
>> bus. This reset will be asserted then released before the downstream
>> devices on the bus are probed.
>>
>> Signed-off-by: Chris Packham <[email protected]>
>> ---
>>
> ...
>
>> Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
>> index fc3dd7ec0445..3f95d71b9985 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
>> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
>> @@ -99,6 +99,14 @@ wants to support one of the below features, it should adapt these bindings.
>> indicates that the system is accessible via this bus as an endpoint for
>> MCTP over I2C transport.
>>
>> +- bus-reset-gpios:
>> + GPIO pin providing a common reset for all downstream devices. This GPIO
>> + will be asserted then released before the downstream devices are probed.
> I initially reviewed it, but did not think enough about it. After more
> consideration, I believe this is not a property of the I2C bus
> controller. This is a property of each device, even if the GPIO is the same.
>
> Linux kernel already supports shared GPIO, so you only need
> enable-ref-counting on it.
That's the kind of breadcrumb I need. Although I can't see
enable-ref-counting as any kind of DT property. Do you mean
GPIOD_FLAGS_BIT_NONEXCLUSIVE?
> Putting it into the controller bindings looks like solving OS issue with
> incorrect hardware description.
Yes that's entirely whats happening here.
> Best regards,
> Krzysztof
>
On 15/11/2023 22:53, Chris Packham wrote:
> Hi Krystof,
>
> On 16/11/23 10:29, Krzysztof Kozlowski wrote:
>> On 15/11/2023 04:57, Chris Packham wrote:
>>> Add bus-reset-gpios and bus-reset-duration-us properties to the binding
>>> description for i2c busses. These can be used to describe hardware where
>>> a common reset GPIO is connected to all downstream devices on and I2C
>>> bus. This reset will be asserted then released before the downstream
>>> devices on the bus are probed.
>>>
>>> Signed-off-by: Chris Packham <[email protected]>
>>> ---
>>>
>> ...
>>
>>> Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> index fc3dd7ec0445..3f95d71b9985 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> @@ -99,6 +99,14 @@ wants to support one of the below features, it should adapt these bindings.
>>> indicates that the system is accessible via this bus as an endpoint for
>>> MCTP over I2C transport.
>>>
>>> +- bus-reset-gpios:
>>> + GPIO pin providing a common reset for all downstream devices. This GPIO
>>> + will be asserted then released before the downstream devices are probed.
>> I initially reviewed it, but did not think enough about it. After more
>> consideration, I believe this is not a property of the I2C bus
>> controller. This is a property of each device, even if the GPIO is the same.
>>
>> Linux kernel already supports shared GPIO, so you only need
>> enable-ref-counting on it.
>
> That's the kind of breadcrumb I need. Although I can't see
> enable-ref-counting as any kind of DT property. Do you mean
> GPIOD_FLAGS_BIT_NONEXCLUSIVE?
It's not a feature or property of Devicetree, but missing feature of OS.
Best regards,
Krzysztof
> > Putting it into the controller bindings looks like solving OS issue with
> > incorrect hardware description.
> Yes that's entirely whats happening here.
So, this series can be dropped?
On 20/12/23 06:02, [email protected] wrote:
>>> Putting it into the controller bindings looks like solving OS issue with
>>> incorrect hardware description.
>> Yes that's entirely whats happening here.
> So, this series can be dropped?
>
I personally would like to see it accepted but it seems there are
objections to this approach. I've yet to come up with anything better to
offer as an alternative.
> I personally would like to see it accepted but it seems there are
> objections to this approach. I've yet to come up with anything better to
> offer as an alternative.
I see. Thanks for the heads up!
Hi,
> > I personally would like to see it accepted but it seems there are
> > objections to this approach. I've yet to come up with anything better to
> > offer as an alternative.
>
> I see. Thanks for the heads up!
I'm also inclined to have this merged. A real fix might take
time.
Myself I have developed a prototype for what has been discussed
with Krzysztof, but I don't know how much time it will take to
get things done.
Andi
On 20/12/2023 00:25, Andi Shyti wrote:
> Hi,
>
>>> I personally would like to see it accepted but it seems there are
>>> objections to this approach. I've yet to come up with anything better to
>>> offer as an alternative.
>>
>> I see. Thanks for the heads up!
>
> I'm also inclined to have this merged. A real fix might take
> time.
NAK
If you intend to merge it, then please carry:
Nacked-by: Krzysztof Kozlowski <[email protected]>
The patchset is wrong and made of wrong reasons. It claimed GPIO cannot
be shared, which is simply not true.
>
> Myself I have developed a prototype for what has been discussed
> with Krzysztof, but I don't know how much time it will take to
> get things done.
Best regards,
Krzysztof
> >>> I personally would like to see it accepted but it seems there are
> >>> objections to this approach. I've yet to come up with anything better to
> >>> offer as an alternative.
> >>
> >> I see. Thanks for the heads up!
> >
> > I'm also inclined to have this merged. A real fix might take
> > time.
>
> NAK
>
> If you intend to merge it, then please carry:
No worries. If this is "abusing" DT, then it is not going to be merged
by me. I am sorry for Chris, but sometimes simple problems create quite
some fuzz because Linux hardware abstractions has not foreseen certain
use cases. Or the APIs dealing with them didn't forsee that. We have
been there a lot of times :/
On 20/12/2023 12:03, [email protected] wrote:
>
>>>>> I personally would like to see it accepted but it seems there are
>>>>> objections to this approach. I've yet to come up with anything better to
>>>>> offer as an alternative.
>>>>
>>>> I see. Thanks for the heads up!
>>>
>>> I'm also inclined to have this merged. A real fix might take
>>> time.
>>
>> NAK
>>
>> If you intend to merge it, then please carry:
>
> No worries. If this is "abusing" DT, then it is not going to be merged
> by me. I am sorry for Chris, but sometimes simple problems create quite
> some fuzz because Linux hardware abstractions has not foreseen certain
> use cases. Or the APIs dealing with them didn't forsee that. We have
> been there a lot of times :/
I need the same solution for WSA884x speaker, for which Mark rejected
simple shared GPIO (even though it would work there), so I am trying to
solve it. It's basically the same case. Now, I am waiting on answer from
Sean Anderson whether he continued his work on reset-gpios controller
from two years ago. Rob wanted handling reset-gpios by generic reset
framework, which would solve these simple cases, here and mine, nicely.
Best regards,
Krzysztof
> from two years ago. Rob wanted handling reset-gpios by generic reset
> framework, which would solve these simple cases, here and mine, nicely.
That sounds good. Good luck with it!
Hi Krzysztof,
On Wed, Dec 20, 2023 at 08:22:38AM +0100, Krzysztof Kozlowski wrote:
> On 20/12/2023 00:25, Andi Shyti wrote:
> > Hi,
> >
> >>> I personally would like to see it accepted but it seems there are
> >>> objections to this approach. I've yet to come up with anything better to
> >>> offer as an alternative.
> >>
> >> I see. Thanks for the heads up!
> >
> > I'm also inclined to have this merged. A real fix might take
> > time.
>
> NAK
>
> If you intend to merge it, then please carry:
>
> Nacked-by: Krzysztof Kozlowski <[email protected]>
ehehe... too much drama here :-)
I know you nacked this patch and of course won't be taken
anywhere.
I was actually referring to Chris previous patch rather than this
one.
Andi
> The patchset is wrong and made of wrong reasons. It claimed GPIO cannot
> be shared, which is simply not true.
>
> >
> > Myself I have developed a prototype for what has been discussed
> > with Krzysztof, but I don't know how much time it will take to
> > get things done.
>
>
> Best regards,
> Krzysztof
>