Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[Andrew Jeffery]

Hi Byron,

I'm Cc'ing openbmc@ on this based on my earlier request (history is at
the bottom)

On Wed, 20 Nov 2019, at 18:34, www wrote:
> Dear Andrew Jeffery，
> 
> Thank you for your reply. I've come up with a plan. Can I trouble you 
> to see if it's ok? The brief introduction is as follows:
> 1. write a pass-through driver; (can switch the pass through function 
> or get the status in user space)
> 2. modify pinctrl driver；（Eliminating the influence of pinctrl on pass 
> through register，At present, the operation of GPIOD/E will affect the 
> pass through function）
> 3. First turn off the pass through function, then turn on or off the 
> host, and then enable the pass through function.
> 
> In this way, there is no need to detect whether GPIO has input or 
> not，and this is the real hardware pass through.
> Do you think this is feasible?

Your proposal sounds like more work than just hooking the two GPIOs
together in userspace, so I need to understand why you're avoiding that
approach. I have some questions:

1. What mode of pass-through are you using? It comes in two flavours:
hardware strapping that affects an entire GPIO bank or paired pass-
through which must be explicitly muxed at runtime.

2. What is the requirement for hardware passthrough here (as
opposed to doing it in userspace when the BMC comes up)? Please
provide as much detail as you can in response so I'm clear on the
motivation.

Another significant issue with your proposal is that removing the
knowledge of pass-through from pinmux means that it's possible to
achieve an inconsistent state between the intent of a pin request and
its resulting behaviour. That's a bug that is currently not possible, so
you will need to do a lot of work to convince me it's the right approach.

Getting back to your problem rather than solutions, it's possible to view
this as a deficiency in the GPIO subsystem and Aspeed GPIO driver: If we
could describe that we want the pin muxed for pass-through as part of
the GPIO request then your problem would be partly resolved, save for 
the fact that the exported GPIO would still be read-only. However, that
issue is fully resolved by multiple sequential GPIO requests: export the
GPIO in pass-through mode initially, and then when it comes to changing
the host state, re-export the GPIO in non-pass-through mode so that it is
writable, and then again re-export the GPIO back in pass-through mode
after the host state change has been applied. This is the sequence of
your original solution above, just without the need for additional drivers
with ad-hoc userspace interfaces or introducing bugs into the pinctrl
driver.

What are your thoughts on this?

A couple of final notes:

a. Aspeed confirmed that GPIOs configured as passthrough are
constrained to read-only behaviour in the GPIO controller. I haven't
tested this to verify, but if it's possible to drive the pin then you'd
want to be mindful of multiple current sources in push-pull or
open/source configurations.

b. Looking at the ast2500 driver the groups for the strapping-based
passthrough aren't described which means some of the mutual-
exclusion behaviour won't quite work as desired. I should fix that.

Cheers,

Andrew

> 
> thanks,
> Byron
> 
> 
> 
> 
> At 2019-11-20 14:52:17, "Andrew Jeffery" <andrew@aj.id.au> wrote:
> >Hi Byron,
> >
> >Sorry about the delayed reply, I have been on leave.
> >
> >Please ask me questions on the list in future, as others may have the same
> >question as you but they can't learn if our exchanges are private :)
> >
> >On Fri, 15 Nov 2019, at 20:06, www wrote:
> >> Dear Andrew Jeffery，
> >> 
> >> I saw that the speed pinctrl driver in the openbmc kernel was uploaded 
> >> by you. 
> >
> >Yep, I maintain the pinctrl drivers for the Aspeed SoCs.
> >
> >> As a result of the test, I found that this driver will affect 
> >> the pass-through function. When operating with GPIO, the pass through 
> >> function will be automatically turned off (this is not the working mode 
> >> of the aspeed chip. On the chip, the two functions can coexist).
> >
> >Only to a point. If the GPIOs are configured for passthrough then accessing
> >the pin via the GPIO controller is read-only according to Aspeed. I'm trying
> >to find where I found this information.
> >
> >> How 
> >> can I modify it to eliminate this effect? I want it to turn on or off 
> >> the system without affecting the passthrough function. Can you give me 
> >> some advice or tell me how to modify it?
> >
> >I understand what you're after. Let me think about it a bit, as it might be
> >possible to cook up a solution. It's complicated by the mutual-exclusion
> >behaviour of the pinctrl subsystem - I want to retain this as it removes an
> >entire class of hard-to-track-down bugs (last-function-probed-wins-the-pin).
> >
> >I assume your system is strapped so that the GPIOs are passthrough? If so, a
> >possible software implementation is to export both of the relevant GPIOs to
> >userspace, and provide a daemon that polls() on the input GPIO and some
> >other external (dbus?) event, and writes the output GPIO when an event
> >occurs. This gives the effect of hardware passthrough with the addition of the
> >arbitrary control over the output GPIO.
> >
> >Hope that helps,
> >
> >Andrew
> 
> 
>

Re:Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[www]

[-- Attachment #1: Type: text/plain, Size: 7069 bytes --]



Dear Andrew，


Thank you for your reply. I'll elaborate my opinion as follows:






At 2019-11-22 08:31:05, "Andrew Jeffery" <andrew@aj.id.au> wrote:
>Hi Byron,
>
>I'm Cc'ing openbmc@ on this based on my earlier request (history is at
>the bottom)
>
>On Wed, 20 Nov 2019, at 18:34, www wrote:
>> Dear Andrew Jeffery，
>> 
>> Thank you for your reply. I've come up with a plan. Can I trouble you 
>> to see if it's ok? The brief introduction is as follows:
>> 1. write a pass-through driver; (can switch the pass through function 
>> or get the status in user space)
>> 2. modify pinctrl driver；（Eliminating the influence of pinctrl on pass 
>> through register，At present, the operation of GPIOD/E will affect the 
>> pass through function）
>> 3. First turn off the pass through function, then turn on or off the 
>> host, and then enable the pass through function.
>> 
>> In this way, there is no need to detect whether GPIO has input or 
>> not，and this is the real hardware pass through.
>> Do you think this is feasible?
>
>Your proposal sounds like more work than just hooking the two GPIOs
>together in userspace, so I need to understand why you're avoiding that
>approach. I have some questions:
>
>1. What mode of pass-through are you using? It comes in two flavours:
>hardware strapping that affects an entire GPIO bank or paired pass-
>through which must be explicitly muxed at runtime.

>


hardware strapping can be modified by software. turn off global pass-through switch, 
and turn on paired pass-through(Set according to your hardware design), 
The specific GPIO setting may need to be obtained from DTS.




>2. What is the requirement for hardware passthrough here (as
>opposed to doing it in userspace when the BMC comes up)? Please
>provide as much detail as you can in response so I'm clear on the
>motivation.

>


The hardware pass through function will be used for power button, reset button or ID button.
(Set according to your hardware design)
When some processes of BMC are hung up and kernel panic appears in BMC, 
even when BMC fails to start(stop at uboot), these buttons can still be used, especially for power button. 
(in this case, you can update the BMC firmware and repair the BMC firmware if you can boot into the system.)




>Another significant issue with your proposal is that removing the
>knowledge of pass-through from pinmux means that it's possible to
>achieve an inconsistent state between the intent of a pin request and
>its resulting behaviour. That's a bug that is currently not possible, so
>you will need to do a lot of work to convince me it's the right approach.



I see that when gpioD/ E is operated, it will first turn off the pass through function, and then perform other operations. 
Because the previous pass through state is not saved, there is no way to restore the pass through state.
I think we should remove the operation of pass thought in gpio driver, 
and then use the special pass through part to open or close the pass through function.
I know this will have a great impact on the GPIO driver part, 
because you are an expert in GPIO direction, so please consult your opinion.




>Getting back to your problem rather than solutions, it's possible to view
>this as a deficiency in the GPIO subsystem and Aspeed GPIO driver: If we
>could describe that we want the pin muxed for pass-through as part of
>the GPIO request then your problem would be partly resolved, save for 
>the fact that the exported GPIO would still be read-only. However, that
>issue is fully resolved by multiple sequential GPIO requests: export the
>GPIO in pass-through mode initially, and then when it comes to changing
>the host state, re-export the GPIO in non-pass-through mode so that it is
>writable, and then again re-export the GPIO back in pass-through mode
>after the host state change has been applied. This is the sequence of
>your original solution above, just without the need for additional drivers
>with ad-hoc userspace interfaces or introducing bugs into the pinctrl
>driver.
>What are your thoughts on this?

>


I haven't heard of this way. Would you please explain it in detail? Thank you




>A couple of final notes:
>
>a. Aspeed confirmed that GPIOs configured as passthrough are
>constrained to read-only behaviour in the GPIO controller. I haven't
>tested this to verify, but if it's possible to drive the pin then you'd
>want to be mindful of multiple current sources in push-pull or
>open/source configurations.

>
In my test, it's true that when enable pass through, the GPIO can only read, not write.




>b. Looking at the ast2500 driver the groups for the strapping-based
>passthrough aren't described which means some of the mutual-
>exclusion behaviour won't quite work as desired. I should fix that.

Thank you for your contribution to this part. I hope you can finish it as soon as possible. Thank you.


thanks,
Byron




>Cheers,
>
>Andrew
>
>> 
>> thanks,
>> Byron
>> 
>> 
>> 
>> 
>> At 2019-11-20 14:52:17, "Andrew Jeffery" <andrew@aj.id.au> wrote:
>> >Hi Byron,
>> >
>> >Sorry about the delayed reply, I have been on leave.
>> >
>> >Please ask me questions on the list in future, as others may have the same
>> >question as you but they can't learn if our exchanges are private :)
>> >
>> >On Fri, 15 Nov 2019, at 20:06, www wrote:
>> >> Dear Andrew Jeffery，
>> >> 
>> >> I saw that the speed pinctrl driver in the openbmc kernel was uploaded 
>> >> by you. 
>> >
>> >Yep, I maintain the pinctrl drivers for the Aspeed SoCs.
>> >
>> >> As a result of the test, I found that this driver will affect 
>> >> the pass-through function. When operating with GPIO, the pass through 
>> >> function will be automatically turned off (this is not the working mode 
>> >> of the aspeed chip. On the chip, the two functions can coexist).
>> >
>> >Only to a point. If the GPIOs are configured for passthrough then accessing
>> >the pin via the GPIO controller is read-only according to Aspeed. I'm trying
>> >to find where I found this information.
>> >
>> >> How 
>> >> can I modify it to eliminate this effect? I want it to turn on or off 
>> >> the system without affecting the passthrough function. Can you give me 
>> >> some advice or tell me how to modify it?
>> >
>> >I understand what you're after. Let me think about it a bit, as it might be
>> >possible to cook up a solution. It's complicated by the mutual-exclusion
>> >behaviour of the pinctrl subsystem - I want to retain this as it removes an
>> >entire class of hard-to-track-down bugs (last-function-probed-wins-the-pin).
>> >
>> >I assume your system is strapped so that the GPIOs are passthrough? If so, a
>> >possible software implementation is to export both of the relevant GPIOs to
>> >userspace, and provide a daemon that polls() on the input GPIO and some
>> >other external (dbus?) event, and writes the output GPIO when an event
>> >occurs. This gives the effect of hardware passthrough with the addition of the
>> >arbitrary control over the output GPIO.
>> >
>> >Hope that helps,
>> >
>> >Andrew
>> 
>> 
>>

[-- Attachment #2: Type: text/html, Size: 8709 bytes --]

Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[Andrew Jeffery]

On Fri, 22 Nov 2019, at 14:55, www wrote:
> At 2019-11-22 08:31:05, "Andrew Jeffery" <andrew@aj.id.au> wrote:

*snip* 

> >Getting back to your problem rather than solutions, it's possible to view
> >this as a deficiency in the GPIO subsystem and Aspeed GPIO driver: If we
> >could describe that we want the pin muxed for pass-through as part of
> >the GPIO request then your problem would be partly resolved, save for 
> >the fact that the exported GPIO would still be read-only. However, that
> >issue is fully resolved by multiple sequential GPIO requests: export the
> >GPIO in pass-through mode initially, and then when it comes to changing
> >the host state, re-export the GPIO in non-pass-through mode so that it is
> >writable, and then again re-export the GPIO back in pass-through mode
> >after the host state change has been applied. This is the sequence of
> >your original solution above, just without the need for additional drivers
> >with ad-hoc userspace interfaces or introducing bugs into the pinctrl
> >driver.
> >What are your thoughts on this?
> >
> 
> I haven't heard of this way. Would you please explain it in detail? Thank you

What details are you after? What I suggested above is not yet possible -
we'd need to develop some kernel patches to make it work, but they would
be something we could upstream. pinctrl needs to remain aware of whether
a pin is in GPIO pass-through mode or not, as it not only affects how that pin
will behave but how *other* "unrelated" pins might behave as well.

Andrew

Re:Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[www]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

Dear Andrew,


Thank you. I got it.
Pass-through function is only a small and special part of GPIO function.
If the entire pinctrl and /sys/class/gpio  are changed due to this function, I am not sure whether it is appropriate.


thanks,
Byron



At 2019-11-22 14:32:51, "Andrew Jeffery" <andrew@aj.id.au> wrote:
>On Fri, 22 Nov 2019, at 14:55, www wrote:
>> At 2019-11-22 08:31:05, "Andrew Jeffery" <andrew@aj.id.au> wrote:
>
>*snip* 
>
>> >Getting back to your problem rather than solutions, it's possible to view
>> >this as a deficiency in the GPIO subsystem and Aspeed GPIO driver: If we
>> >could describe that we want the pin muxed for pass-through as part of
>> >the GPIO request then your problem would be partly resolved, save for 
>> >the fact that the exported GPIO would still be read-only. However, that
>> >issue is fully resolved by multiple sequential GPIO requests: export the
>> >GPIO in pass-through mode initially, and then when it comes to changing
>> >the host state, re-export the GPIO in non-pass-through mode so that it is
>> >writable, and then again re-export the GPIO back in pass-through mode
>> >after the host state change has been applied. This is the sequence of
>> >your original solution above, just without the need for additional drivers
>> >with ad-hoc userspace interfaces or introducing bugs into the pinctrl
>> >driver.
>> >What are your thoughts on this?
>> >
>> 
>> I haven't heard of this way. Would you please explain it in detail? Thank you
>
>What details are you after? What I suggested above is not yet possible -
>we'd need to develop some kernel patches to make it work, but they would
>be something we could upstream. pinctrl needs to remain aware of whether
>a pin is in GPIO pass-through mode or not, as it not only affects how that pin
>will behave but how *other* "unrelated" pins might behave as well.
>
>Andrew

[-- Attachment #2: Type: text/html, Size: 2967 bytes --]

Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[Andrew Jeffery]

On Fri, 22 Nov 2019, at 17:20, www wrote:
> Dear Andrew,
> 
> Thank you. I got it.
> Pass-through function is only a small and special part of GPIO 
> function. 
> If the entire pinctrl and ****/sys/class/gpio **** are changed due to 
> this function, I am not sure whether it is appropriate.
> 

I'm not suggesting you change pinctrl at all, so I'm not sure where you
got the idea that we'd need something so drastic as changing entire
subsystems. What I'm proposing should boil down to a new
configuration option to pass through the gpio userspace interface,
then adding appropriate support to the GPIO driver. We might need
to tinker with pinctrl a little to get the interfaces right, but nothing more.

Andrew

Re:Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[www]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

hi Andrew,


This suggestion is my personal imperfect idea. It's also the easiest way to do it. It may not match the requirements of the kernel organization, and it may be necessary to find a more perfect method.


thanks,
Byron






At 2019-11-25 06:10:37, "Andrew Jeffery" <andrew@aj.id.au> wrote:
>
>
>On Fri, 22 Nov 2019, at 17:20, www wrote:
>> Dear Andrew,
>> 
>> Thank you. I got it.
>> Pass-through function is only a small and special part of GPIO 
>> function. 
>> If the entire pinctrl and ****/sys/class/gpio **** are changed due to 
>> this function, I am not sure whether it is appropriate.
>> 
>
>I'm not suggesting you change pinctrl at all, so I'm not sure where you
>got the idea that we'd need something so drastic as changing entire
>subsystems. What I'm proposing should boil down to a new
>configuration option to pass through the gpio userspace interface,
>then adding appropriate support to the GPIO driver. We might need
>to tinker with pinctrl a little to get the interfaces right, but nothing more.
>
>Andrew

[-- Attachment #2: Type: text/html, Size: 1623 bytes --]

Re: [openbmc-kernel]: How to make pinctrl not affect pass-through function?

[Andrew Jeffery]

On Tue, 26 Nov 2019, at 11:30, www wrote:
> hi Andrew,
> 
> This suggestion is my personal imperfect idea. It's also the easiest 
> way to do it. It may not match the requirements of the kernel 
> organization, and it may be necessary to find a more perfect method.
> 

Right, if you're going to do it as an out-of-tree hack then there's nothing
I can do to prevent you :)

I'd prefer to stick to discussing solutions we can upstream.

Cheers,

Andrew