* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 13:16 ` Vladimir Oltean
@ 2025-12-04 13:50 ` Daniel Golle
2025-12-04 15:45 ` Vladimir Oltean
2025-12-04 14:49 ` Krzysztof Kozlowski
2025-12-04 15:22 ` Andrew Lunn
2 siblings, 1 reply; 28+ messages in thread
From: Daniel Golle @ 2025-12-04 13:50 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Frank Wunderlich, Krzysztof Kozlowski, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On Thu, Dec 04, 2025 at 03:16:26PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:
> > On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
> > > Hi,
> > >
> > > Am 01.12.25 um 08:48 schrieb Krzysztof Kozlowski:
> > > > On 30/11/2025 21:17, Andrew Lunn wrote:
> > > > > On Sun, Nov 30, 2025 at 10:07:31AM +0200, Vladimir Oltean wrote:
> > > > > > On Sun, Nov 30, 2025 at 02:11:05AM +0100, Andrew Lunn wrote:
> > > > > > > > - gpiod_set_value_cansleep(priv->reset, 0);
> > > > > > > > + int is_active_low = !!gpiod_is_active_low(priv->reset);
> > > > > > > > + gpiod_set_value_cansleep(priv->reset, is_active_low);
> > > > > > > I think you did not correctly understand what Russell said. You pass
> > > > > > > the logical value to gpiod_set_value(). If the GPIO has been marked as
> > > > > > > active LOW, the GPIO core will invert the logical values to the raw
> > > > > > > value. You should not be using gpiod_is_active_low().
> > > > > > >
> > > > > > > But as i said to the previous patch, i would just leave everything as
> > > > > > > it is, except document the issue.
> > > > > > >
> > > > > > > Andrew
> > > > > > >
> > > > > > It was my suggestion to do it like this (but I don't understand why I'm
> > > > > > again not in CC).
> > > > > >
> > > > > > We _know_ that the reset pin of the switch should be active low. So by
> > > > > > using gpiod_is_active_low(), we can determine whether the device tree is
> > > > > > wrong or not, and we can work with a wrong device tree too (just invert
> > > > > > the logical values).
> > > > > Assuming there is not a NOT gate placed between the GPIO and the reset
> > > > > pin, because the board designer decided to do that for some reason?
> > > jumping in because i prepare mt7987 / BPI-R4Lite dts for upstreaming when
> > > driver-changes are in.
> > > With current driver i need to define the reset-gpio for mt7531 again wrong
> > > to get it
> > > working. So to have future dts correct, imho this (or similar) change to
> > > driver is needed.
> > >
> > > Of course we cannot simply say that current value is wrong and just invert
> > > it because of
> > > possible "external" inversion of reset signal between SoC and switch.
> > > I have to look on schematics for the boards i have (BPI-R64, BPI-R3,
> > > BPI-R2Pro) if there is such circuit.
> >
> > I'm also not aware of any board which doesn't directly connect the
> > reset of the MT7530 to a GPIO pin of the SoC. For MediaTek's designs
> > there is often even a specific pin desginated for this purpose and
> > most vendors do follow this. If they deviate at all, then it's just
> > that a different pin is used for the switch reset, but I've never
> > seen any logic between the SoC's GPIO pin and the switch reset.
> >
> > > Maybe the mt7988 (mt7530-mmio) based boards also affected?
> >
> > There is no GPIO reset for switches which are integrated in the SoC,
> > so this only matters for external MT7530 and MT7531 ICs for which an
> > actual GPIO line connected to the SoC is used to reset the switch.
>
> I get the feeling that we're complicating a simple solution because of a
> theoretical "what if" scenario. The "NOT" gate is somewhat contrived
> given the fact that most GPIOs can already be active high or low, but OK.
>
> If this is blocking progress for new device trees, can we just construct,
> using of_machine_is_compatible(), a list of all boards where the device
> tree defines incorrect reset polarity that shouldn't be trusted by the
> driver when driving the reset GPIO? If we do this, we can also leave
> those existing device trees alone.
From OpenWrt's point of view this would be kind of ugly as we would either
have to extend the list of affected boards downstream, or fix the polarity
in some but not all of our downstream DTS files. I'd prefer to rather
have the option to force the "wrong" GPIO polarity for theoretical future
boards with that (very unlikely to ever exist) NOT gate between the SoC
GPIO and switch reset line. That would allow to gradually update boards
to reflect the physical reality and yet the driver would not break if the
GPIO polarity is stated wrongly.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 13:50 ` Daniel Golle
@ 2025-12-04 15:45 ` Vladimir Oltean
0 siblings, 0 replies; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 15:45 UTC (permalink / raw)
To: Daniel Golle
Cc: Frank Wunderlich, Krzysztof Kozlowski, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On Thu, Dec 04, 2025 at 01:50:52PM +0000, Daniel Golle wrote:
> On Thu, Dec 04, 2025 at 03:16:26PM +0200, Vladimir Oltean wrote:
> > If this is blocking progress for new device trees, can we just construct,
> > using of_machine_is_compatible(), a list of all boards where the device
> > tree defines incorrect reset polarity that shouldn't be trusted by the
> > driver when driving the reset GPIO? If we do this, we can also leave
> > those existing device trees alone.
>
> From OpenWrt's point of view this would be kind of ugly as we would either
> have to extend the list of affected boards downstream, or fix the polarity
> in some but not all of our downstream DTS files.
Including the downstream-only boards, how many compatibles are we
talking about? If we patched mainline to cover all, are you confident
that it would be an exhaustive solution?
> I'd prefer to rather have the option to force the "wrong" GPIO
> polarity for theoretical future boards with that (very unlikely to
> ever exist) NOT gate between the SoC GPIO and switch reset line. That
> would allow to gradually update boards to reflect the physical reality
> and yet the driver would not break if the GPIO polarity is stated
> wrongly.
I didn't get that from this thread - how would you prefer having this
option implemented exactly?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 13:16 ` Vladimir Oltean
2025-12-04 13:50 ` Daniel Golle
@ 2025-12-04 14:49 ` Krzysztof Kozlowski
2025-12-04 16:02 ` Vladimir Oltean
2025-12-04 15:22 ` Andrew Lunn
2 siblings, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-04 14:49 UTC (permalink / raw)
To: Vladimir Oltean, Daniel Golle
Cc: Frank Wunderlich, Andrew Lunn, Chen Minqiang, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On 04/12/2025 14:16, Vladimir Oltean wrote:
> On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:
>> On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
>>> Hi,
>>>
>>> Am 01.12.25 um 08:48 schrieb Krzysztof Kozlowski:
>>>> On 30/11/2025 21:17, Andrew Lunn wrote:
>>>>> On Sun, Nov 30, 2025 at 10:07:31AM +0200, Vladimir Oltean wrote:
>>>>>> On Sun, Nov 30, 2025 at 02:11:05AM +0100, Andrew Lunn wrote:
>>>>>>>> - gpiod_set_value_cansleep(priv->reset, 0);
>>>>>>>> + int is_active_low = !!gpiod_is_active_low(priv->reset);
>>>>>>>> + gpiod_set_value_cansleep(priv->reset, is_active_low);
>>>>>>> I think you did not correctly understand what Russell said. You pass
>>>>>>> the logical value to gpiod_set_value(). If the GPIO has been marked as
>>>>>>> active LOW, the GPIO core will invert the logical values to the raw
>>>>>>> value. You should not be using gpiod_is_active_low().
>>>>>>>
>>>>>>> But as i said to the previous patch, i would just leave everything as
>>>>>>> it is, except document the issue.
>>>>>>>
>>>>>>> Andrew
>>>>>>>
>>>>>> It was my suggestion to do it like this (but I don't understand why I'm
>>>>>> again not in CC).
>>>>>>
>>>>>> We _know_ that the reset pin of the switch should be active low. So by
>>>>>> using gpiod_is_active_low(), we can determine whether the device tree is
>>>>>> wrong or not, and we can work with a wrong device tree too (just invert
>>>>>> the logical values).
>>>>> Assuming there is not a NOT gate placed between the GPIO and the reset
>>>>> pin, because the board designer decided to do that for some reason?
>>> jumping in because i prepare mt7987 / BPI-R4Lite dts for upstreaming when
>>> driver-changes are in.
>>> With current driver i need to define the reset-gpio for mt7531 again wrong
>>> to get it
>>> working. So to have future dts correct, imho this (or similar) change to
>>> driver is needed.
>>>
>>> Of course we cannot simply say that current value is wrong and just invert
>>> it because of
>>> possible "external" inversion of reset signal between SoC and switch.
>>> I have to look on schematics for the boards i have (BPI-R64, BPI-R3,
>>> BPI-R2Pro) if there is such circuit.
>>
>> I'm also not aware of any board which doesn't directly connect the
>> reset of the MT7530 to a GPIO pin of the SoC. For MediaTek's designs
>> there is often even a specific pin desginated for this purpose and
>> most vendors do follow this. If they deviate at all, then it's just
>> that a different pin is used for the switch reset, but I've never
>> seen any logic between the SoC's GPIO pin and the switch reset.
>>
>>> Maybe the mt7988 (mt7530-mmio) based boards also affected?
>>
>> There is no GPIO reset for switches which are integrated in the SoC,
>> so this only matters for external MT7530 and MT7531 ICs for which an
>> actual GPIO line connected to the SoC is used to reset the switch.
>
> I get the feeling that we're complicating a simple solution because of a
> theoretical "what if" scenario. The "NOT" gate is somewhat contrived
You downplay this case and suggest (if I get it right) that NOT gate is
something unusual.
I mentioned "line inverter" but it's not about NOT gate. There is no
need for NOT gate at all, like some magical component which no one puts
to the board. The only thing needed is just to pull the GPIO up or down,
that's it. It's completely normal design thus it CAN happen.
Of course "can" does not mean it actually does, because certain
configurations like powerdown-fail-safe are more likely and I am not an
electric circuit designer to tell which one is better, but that
downplaying does not help here.
Just to clarify: I expect clear communication that some users will be
broken with as good as you can provide analysis of the impact (which
users). I only object the clame here "no one can ever pull down a GPIO
line thus I handled all possible cases and made it backward compatible".
And that claim to quote was:
"Therefore, regardless of whether a DTS is old or new, correct or
incorrect, the driver now generates the correct electrical reset pulse."
which is 100% false and I am surprised how one could claim that.
> given the fact that most GPIOs can already be active high or low, but OK.
>
> If this is blocking progress for new device trees, can we just construct,
> using of_machine_is_compatible(), a list of all boards where the device
> tree defines incorrect reset polarity that shouldn't be trusted by the
> driver when driving the reset GPIO? If we do this, we can also leave
> those existing device trees alone.
Works for me. You can also print fat warning for every unusual case.
Nothing like that was here in the original patch...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 14:49 ` Krzysztof Kozlowski
@ 2025-12-04 16:02 ` Vladimir Oltean
2025-12-04 16:48 ` Krzysztof Kozlowski
0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 16:02 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Daniel Golle, Frank Wunderlich, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On Thu, Dec 04, 2025 at 03:49:52PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 14:16, Vladimir Oltean wrote:
> > I get the feeling that we're complicating a simple solution because of a
> > theoretical "what if" scenario. The "NOT" gate is somewhat contrived
>
> You downplay this case and suggest (if I get it right) that NOT gate is
> something unusual.
>
> I mentioned "line inverter" but it's not about NOT gate. There is no
> need for NOT gate at all, like some magical component which no one puts
> to the board. The only thing needed is just to pull the GPIO up or down,
> that's it. It's completely normal design thus it CAN happen.
>
> Of course "can" does not mean it actually does, because certain
> configurations like powerdown-fail-safe are more likely and I am not an
> electric circuit designer to tell which one is better, but that
> downplaying does not help here.
I don't want to dismiss this comment, but I don't really understand it.
What do you mean by "line inverter", is it the component inside the GPIO
pin which makes it active low?
I thought that the premise of this patch set is that old device trees
are all (incorrectly) defined as GPIO_ACTIVE_HIGH, but someone familiar
with the matter needs to fact-check this statement.
Anyway, you and Andrew are talking about different things, you haven't
made it clear (or at least it wasn't clear to me) that the inverter you
are talking about isn't his NOT gate (that isn't described in the device
tree at all, as opposed to your inverter which would make the GPIO line
GPIO_ACTIVE_LOW - that's something verifiable).
> Just to clarify: I expect clear communication that some users will be
> broken with as good as you can provide analysis of the impact (which
> users). I only object the clame here "no one can ever pull down a GPIO
> line thus I handled all possible cases and made it backward compatible".
>
> And that claim to quote was:
> "Therefore, regardless of whether a DTS is old or new, correct or
> incorrect, the driver now generates the correct electrical reset pulse."
>
> which is 100% false and I am surprised how one could claim that.
Agree, the communication should be better.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 16:02 ` Vladimir Oltean
@ 2025-12-04 16:48 ` Krzysztof Kozlowski
2025-12-04 17:11 ` Vladimir Oltean
0 siblings, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-04 16:48 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, Frank Wunderlich, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On 04/12/2025 17:02, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 03:49:52PM +0100, Krzysztof Kozlowski wrote:
>> On 04/12/2025 14:16, Vladimir Oltean wrote:
>>> I get the feeling that we're complicating a simple solution because of a
>>> theoretical "what if" scenario. The "NOT" gate is somewhat contrived
>>
>> You downplay this case and suggest (if I get it right) that NOT gate is
>> something unusual.
>>
>> I mentioned "line inverter" but it's not about NOT gate. There is no
>> need for NOT gate at all, like some magical component which no one puts
>> to the board. The only thing needed is just to pull the GPIO up or down,
>> that's it. It's completely normal design thus it CAN happen.
>>
>> Of course "can" does not mean it actually does, because certain
>> configurations like powerdown-fail-safe are more likely and I am not an
>> electric circuit designer to tell which one is better, but that
>> downplaying does not help here.
>
> I don't want to dismiss this comment, but I don't really understand it.
> What do you mean by "line inverter", is it the component inside the GPIO
> pin which makes it active low?
I mentioned much earlier "line inverter" and here the "NOT gate"
appeared and both suggest you need some dedicated component.
What I want to say that it is not true. You do not need dedicated
component to have different polarity, because it is as simple as
connecting the wire to ground or to voltage and moving a resistor from
one place to another.
>
> I thought that the premise of this patch set is that old device trees
> are all (incorrectly) defined as GPIO_ACTIVE_HIGH, but someone familiar
> with the matter needs to fact-check this statement.
Yes, that's most likely true.
>
> Anyway, you and Andrew are talking about different things, you haven't
> made it clear (or at least it wasn't clear to me) that the inverter you
> are talking about isn't his NOT gate (that isn't described in the device
> tree at all, as opposed to your inverter which would make the GPIO line
> GPIO_ACTIVE_LOW - that's something verifiable).
Both are the same - inverter or NOT gate, same stuff. It is just
connecting wire to pull up, not actual component on the board (although
one could make and buy such component as well...). We never describe
these inverters in the DTS, these are just too trivial circuits, thus
the final GPIO_ACTIVE_XXX should already include whatever is on the wire
between SoC and device.
>
>> Just to clarify: I expect clear communication that some users will be
>> broken with as good as you can provide analysis of the impact (which
>> users). I only object the clame here "no one can ever pull down a GPIO
>> line thus I handled all possible cases and made it backward compatible".
>>
>> And that claim to quote was:
>> "Therefore, regardless of whether a DTS is old or new, correct or
>> incorrect, the driver now generates the correct electrical reset pulse."
>>
>> which is 100% false and I am surprised how one could claim that.
>
> Agree, the communication should be better.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 16:48 ` Krzysztof Kozlowski
@ 2025-12-04 17:11 ` Vladimir Oltean
2025-12-04 17:23 ` Krzysztof Kozlowski
0 siblings, 1 reply; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 17:11 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Daniel Golle, Frank Wunderlich, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
> Both are the same - inverter or NOT gate, same stuff. It is just
> connecting wire to pull up, not actual component on the board (although
> one could make and buy such component as well...). We never describe
> these inverters in the DTS, these are just too trivial circuits, thus
> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
> between SoC and device.
Please read what Andrew said:
https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
Assuming there is not a NOT gate placed between the GPIO and the reset
pin, because the board designer decided to do that for some reason?
~~~~~~~~~~~~~~
You two are *not* talking about the same thing. I dismissed the
probability of there being a NOT gate in the form of a discrete chip on
the PCB, *exactly because* you can most likely invert the signal in the
GPIO pin itself.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 17:11 ` Vladimir Oltean
@ 2025-12-04 17:23 ` Krzysztof Kozlowski
2025-12-04 17:32 ` Krzysztof Kozlowski
2025-12-04 17:45 ` Russell King (Oracle)
0 siblings, 2 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-04 17:23 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, Frank Wunderlich, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On 04/12/2025 18:11, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
>> Both are the same - inverter or NOT gate, same stuff. It is just
>> connecting wire to pull up, not actual component on the board (although
>> one could make and buy such component as well...). We never describe
>> these inverters in the DTS, these are just too trivial circuits, thus
>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
>> between SoC and device.
>
> Please read what Andrew said:
> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
>
> Assuming there is not a NOT gate placed between the GPIO and the reset
> pin, because the board designer decided to do that for some reason?
> ~~~~~~~~~~~~~~
>
> You two are *not* talking about the same thing. I dismissed the
It's the same thing. NOT gate is just pulling some pin down or up.
> probability of there being a NOT gate in the form of a discrete chip on
We do not describe NOT gates as discreet chips. I don't think anyone
actually places something as NOT gate. It's logical NOT gate, but on
circuit it is just pull up/down as I said multiple times. The pull +
resistor is the "NOT gate".
It's so easy and that's why it is potentially so common design.
> the PCB, *exactly because* you can most likely invert the signal in the
> GPIO pin itself.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 17:23 ` Krzysztof Kozlowski
@ 2025-12-04 17:32 ` Krzysztof Kozlowski
2025-12-04 20:47 ` Russell King (Oracle)
2025-12-04 17:45 ` Russell King (Oracle)
1 sibling, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-04 17:32 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, Frank Wunderlich, Andrew Lunn, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On 04/12/2025 18:23, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:11, Vladimir Oltean wrote:
>> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
>>> Both are the same - inverter or NOT gate, same stuff. It is just
>>> connecting wire to pull up, not actual component on the board (although
>>> one could make and buy such component as well...). We never describe
>>> these inverters in the DTS, these are just too trivial circuits, thus
>>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
>>> between SoC and device.
>>
>> Please read what Andrew said:
>> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
>>
>> Assuming there is not a NOT gate placed between the GPIO and the reset
>> pin, because the board designer decided to do that for some reason?
>> ~~~~~~~~~~~~~~
>>
>> You two are *not* talking about the same thing. I dismissed the
>
>
> It's the same thing. NOT gate is just pulling some pin down or up.
Although transistor would be still needed, so indeed that's still a bit
more than a wire and resistor as I implied.
It looks like:
https://www.electronics-tutorials.ws/wp-content/uploads/2018/05/logic-log47.gif
>
>> probability of there being a NOT gate in the form of a discrete chip on
>
> We do not describe NOT gates as discreet chips. I don't think anyone
> actually places something as NOT gate. It's logical NOT gate, but on
> circuit it is just pull up/down as I said multiple times. The pull +
> resistor is the "NOT gate".
>
> It's so easy and that's why it is potentially so common design.
We still do not describe transistors in DTS. We do not describe NOT
gates. The final flag should include that as I said.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 17:32 ` Krzysztof Kozlowski
@ 2025-12-04 20:47 ` Russell King (Oracle)
0 siblings, 0 replies; 28+ messages in thread
From: Russell King (Oracle) @ 2025-12-04 20:47 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vladimir Oltean, Daniel Golle, Frank Wunderlich, Andrew Lunn,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
On Thu, Dec 04, 2025 at 06:32:23PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:23, Krzysztof Kozlowski wrote:
> > On 04/12/2025 18:11, Vladimir Oltean wrote:
> >> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
> >>> Both are the same - inverter or NOT gate, same stuff. It is just
> >>> connecting wire to pull up, not actual component on the board (although
> >>> one could make and buy such component as well...). We never describe
> >>> these inverters in the DTS, these are just too trivial circuits, thus
> >>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
> >>> between SoC and device.
> >>
> >> Please read what Andrew said:
> >> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
> >>
> >> Assuming there is not a NOT gate placed between the GPIO and the reset
> >> pin, because the board designer decided to do that for some reason?
> >> ~~~~~~~~~~~~~~
> >>
> >> You two are *not* talking about the same thing. I dismissed the
> >
> >
> > It's the same thing. NOT gate is just pulling some pin down or up.
>
> Although transistor would be still needed, so indeed that's still a bit
> more than a wire and resistor as I implied.
>
> It looks like:
> https://www.electronics-tutorials.ws/wp-content/uploads/2018/05/logic-log47.gif
A bit more than even that... I do hope folk don't use exactly that,
that's the kind of thing that would be used in "learning about
electronics" projects! That's a recipe to drive a transistor into
full saturation, which makes it comparitively very slow to turn off.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 17:23 ` Krzysztof Kozlowski
2025-12-04 17:32 ` Krzysztof Kozlowski
@ 2025-12-04 17:45 ` Russell King (Oracle)
2025-12-04 18:21 ` Vladimir Oltean
1 sibling, 1 reply; 28+ messages in thread
From: Russell King (Oracle) @ 2025-12-04 17:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vladimir Oltean, Daniel Golle, Frank Wunderlich, Andrew Lunn,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
On Thu, Dec 04, 2025 at 06:23:48PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:11, Vladimir Oltean wrote:
> > You two are *not* talking about the same thing. I dismissed the
>
> It's the same thing. NOT gate is just pulling some pin down or up.
>
> > probability of there being a NOT gate in the form of a discrete chip on
>
> We do not describe NOT gates as discreet chips. I don't think anyone
> actually places something as NOT gate. It's logical NOT gate, but on
> circuit it is just pull up/down as I said multiple times. The pull +
> resistor is the "NOT gate".
You can get SOT-23 packages that are NOT gates. E.g.
https://www.ti.com/lit/ds/symlink/sn74ahc1g04.pdf?ts=1764851465989
This is a real NOT gate - it's absolute max ratings state that it
can source/sink up to 25mA through its output. So no external
resistor required.
What you describe sounds to me more like a normal transistor though.
Irrespective of this, the exact nature of a device that inverts the
level of a reset signal in the path between a GPIO pin and a device
is irrelevant. What matters is whether there is or is not such a
device.
So, can we stop splitting hairs about what a NOT gate is in this
discussion? It's irrelevant.
> It's so easy and that's why it is potentially so common design.
Do you have any real evidence of this when used with a GPIO pin for
a reset input?
It would make sense if a single GPIO pin is used for resetting
several devices, some of them with an active-high reset input and
others with active-low.
What matters for a GPIO pin used to source a reset signal is "what
is the active level at the GPIO pin for the reset to be asserted to
the connected device(s)."
If we have a device that requires an active-low reset input, but there
is some form of inversion in the path to that input from a GPIO, then
the GPIO _should_ be marked active-high. If the same active-low reset
input is connected directly to a GPIO, the GPIO _should_ be marked as
active-low. Thus, to assert reset, writing '1' through
gpiod_set_value() _should_ assert the reset input on the target device
in both cases.
This is why gpiolib supports software inversion - so software engineers
can think in terms of positive non-inverted logic when programming
GPIOs.
Sadly, we keep having people mark active-low signals as "active high"
in DT, and then have to write '0' to assert the signal. These people
basically don't understand electronics and/or our GPIO model.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 17:45 ` Russell King (Oracle)
@ 2025-12-04 18:21 ` Vladimir Oltean
0 siblings, 0 replies; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 18:21 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Krzysztof Kozlowski, Daniel Golle, Frank Wunderlich, Andrew Lunn,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
On Thu, Dec 04, 2025 at 05:45:30PM +0000, Russell King (Oracle) wrote:
> It would make sense if a single GPIO pin is used for resetting
> several devices, some of them with an active-high reset input and
> others with active-low.
>
> What matters for a GPIO pin used to source a reset signal is "what
> is the active level at the GPIO pin for the reset to be asserted to
> the connected device(s)."
>
> If we have a device that requires an active-low reset input, but there
> is some form of inversion in the path to that input from a GPIO, then
> the GPIO _should_ be marked active-high. If the same active-low reset
> input is connected directly to a GPIO, the GPIO _should_ be marked as
> active-low. Thus, to assert reset, writing '1' through
> gpiod_set_value() _should_ assert the reset input on the target device
> in both cases.
>
> This is why gpiolib supports software inversion - so software engineers
> can think in terms of positive non-inverted logic when programming
> GPIOs.
Thanks for this message, to me it brought new info which I didn't
consider before, which is that if there are *future* boards where some
sort of inversion on the reset signal (likely due to it being shared as
you say), they should be described as GPIO_ACTIVE_HIGH, but this patch
locks us out of that possibility.
If I may bring one more data point into the discussion: the switch
binding specifically requests not to describe shared reset lines
(because unrelated drivers might reset the switch after it probed):
reset-gpios:
description: |
GPIO to reset the switch. Use this if mediatek,mcm is not used.
This property is optional because some boards share the reset line with
other components which makes it impossible to probe the switch if the
reset line is used.
so I guess that the probability for a reset signal inversion, even if
it exists, to cause problems in the GPIO_ACTIVE_HIGH reinterpretation,
is still low, albeit for a completely different reason than the one I
initially claimed (which is bogus and Krzysztof was right to challenge it).
> Sadly, we keep having people mark active-low signals as "active high"
> in DT, and then have to write '0' to assert the signal. These people
> basically don't understand electronics and/or our GPIO model.
This is the case we seem to be in.
On a scale of safety, having quirks for the affected device trees still
ranks a bit higher, though.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 13:16 ` Vladimir Oltean
2025-12-04 13:50 ` Daniel Golle
2025-12-04 14:49 ` Krzysztof Kozlowski
@ 2025-12-04 15:22 ` Andrew Lunn
2025-12-04 15:37 ` Vladimir Oltean
2 siblings, 1 reply; 28+ messages in thread
From: Andrew Lunn @ 2025-12-04 15:22 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, Frank Wunderlich, Krzysztof Kozlowski,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
> If this is blocking progress for new device trees, can we just construct,
> using of_machine_is_compatible(), a list of all boards where the device
> tree defines incorrect reset polarity that shouldn't be trusted by the
> driver when driving the reset GPIO? If we do this, we can also leave
> those existing device trees alone.
I've still not seen a good answer to my question, why not just leave
it 'broken', and document the fact.
Does the fact it is inverted in both DT and the driver prevent us from
making some board work?
Why do we need to fix this?
Sometimes it is better to just leave it alone, if it is not hurting
anybody.
Andrew
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 15:22 ` Andrew Lunn
@ 2025-12-04 15:37 ` Vladimir Oltean
2025-12-04 15:50 ` Andrew Lunn
2025-12-04 15:52 ` Krzysztof Kozlowski
0 siblings, 2 replies; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 15:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: Daniel Golle, Frank Wunderlich, Krzysztof Kozlowski,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
On Thu, Dec 04, 2025 at 04:22:10PM +0100, Andrew Lunn wrote:
> > If this is blocking progress for new device trees, can we just construct,
> > using of_machine_is_compatible(), a list of all boards where the device
> > tree defines incorrect reset polarity that shouldn't be trusted by the
> > driver when driving the reset GPIO? If we do this, we can also leave
> > those existing device trees alone.
>
> I've still not seen a good answer to my question, why not just leave
> it 'broken', and document the fact.
>
> Does the fact it is inverted in both DT and the driver prevent us from
> making some board work?
>
> Why do we need to fix this?
>
> Sometimes it is better to just leave it alone, if it is not hurting
> anybody.
>
> Andrew
Frank said that the fact the driver expecting a wrong device tree is
forcing him to keep introducing even more wrong device trees for new
boards.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 15:37 ` Vladimir Oltean
@ 2025-12-04 15:50 ` Andrew Lunn
2025-12-04 15:52 ` Krzysztof Kozlowski
1 sibling, 0 replies; 28+ messages in thread
From: Andrew Lunn @ 2025-12-04 15:50 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Daniel Golle, Frank Wunderlich, Krzysztof Kozlowski,
Chen Minqiang, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chester A. Unal,
DENG Qingfang, Sean Wang, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
> Frank said that the fact the driver expecting a wrong device tree is
> forcing him to keep introducing even more wrong device trees for new
> boards.
So it sounds like there is no technical reason to fix this, it is just
aesthetics?
Andrew
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 15:37 ` Vladimir Oltean
2025-12-04 15:50 ` Andrew Lunn
@ 2025-12-04 15:52 ` Krzysztof Kozlowski
2025-12-04 16:33 ` Vladimir Oltean
1 sibling, 1 reply; 28+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-04 15:52 UTC (permalink / raw)
To: Vladimir Oltean, Andrew Lunn
Cc: Daniel Golle, Frank Wunderlich, Chen Minqiang, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On 04/12/2025 16:37, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 04:22:10PM +0100, Andrew Lunn wrote:
>>> If this is blocking progress for new device trees, can we just construct,
>>> using of_machine_is_compatible(), a list of all boards where the device
>>> tree defines incorrect reset polarity that shouldn't be trusted by the
>>> driver when driving the reset GPIO? If we do this, we can also leave
>>> those existing device trees alone.
>>
>> I've still not seen a good answer to my question, why not just leave
>> it 'broken', and document the fact.
>>
>> Does the fact it is inverted in both DT and the driver prevent us from
>> making some board work?
>>
>> Why do we need to fix this?
>>
>> Sometimes it is better to just leave it alone, if it is not hurting
>> anybody.
>>
>> Andrew
>
> Frank said that the fact the driver expecting a wrong device tree is
> forcing him to keep introducing even more wrong device trees for new
> boards.
Yeah. BTW, you can also refer to one of my commits -
738455858a2d21b769f673892546cf8300c9fd78 - but also note that similar
work later combined with much more useful change of gpio->gpiod API was
rejected by Mark Brown on basis:
"No, the DT is supposed to be an ABI. The point in having a domain
specific language with a compiler is to allow device trees to be
distributed independently of the kernel."
https://lore.kernel.org/all/9942c3a9-51d1-4161-8871-f6ec696cb4db@sirena.org.uk/
What's interesting, exactly the same commit for the same file, done by
different person (Peng), introducing the same issues without addressing
them, was then merged by Mark:
https://patch.msgid.link/20250324-wcd-gpiod-v2-2-773f67ce3b56@nxp.com
(commit c2d359b4acfbe847d3edcc25d3dc4e594daf9010)
so you know... it's all relative. :)
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence
2025-12-04 15:52 ` Krzysztof Kozlowski
@ 2025-12-04 16:33 ` Vladimir Oltean
0 siblings, 0 replies; 28+ messages in thread
From: Vladimir Oltean @ 2025-12-04 16:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andrew Lunn, Daniel Golle, Frank Wunderlich, Chen Minqiang,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Chester A. Unal, DENG Qingfang,
Sean Wang, linux-kernel, linux-arm-kernel, linux-mediatek, netdev
On Thu, Dec 04, 2025 at 04:52:53PM +0100, Krzysztof Kozlowski wrote:
> Yeah. BTW, you can also refer to one of my commits -
> 738455858a2d21b769f673892546cf8300c9fd78 - but also note that similar
> work later combined with much more useful change of gpio->gpiod API was
> rejected by Mark Brown on basis:
>
> "No, the DT is supposed to be an ABI. The point in having a domain
> specific language with a compiler is to allow device trees to be
> distributed independently of the kernel."
>
> https://lore.kernel.org/all/9942c3a9-51d1-4161-8871-f6ec696cb4db@sirena.org.uk/
>
> What's interesting, exactly the same commit for the same file, done by
> different person (Peng), introducing the same issues without addressing
> them, was then merged by Mark:
> https://patch.msgid.link/20250324-wcd-gpiod-v2-2-773f67ce3b56@nxp.com
> (commit c2d359b4acfbe847d3edcc25d3dc4e594daf9010)
>
> so you know... it's all relative. :)
Yeah, IDK. May we all stand by our own past statements.
^ permalink raw reply [flat|nested] 28+ messages in thread
