| Message ID | 20250424062017.652969-1-uwu@icenowy.me |
|---|---|
| Headers | show |
| Series | pinctrl: starfive: jh7110: support force inputs | expand |
On Thu, Apr 24, 2025 at 8:20 AM Icenowy Zheng <uwu@icenowy.me> wrote: > The JH7110 SoC could support internal GPI signals to be routed to not > external GPIO but internal low/high levels. > > Add two macros, PAD_INTERNAL_LOW and PAD_INTERNAL_HIGH, as two virtual > "pads" to represent internal GPI sources with fixed low/high levels. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> As per my other reply in the previous post, I think this should be handled internal in the kernel instead using a tighter integration between the GPIO and pin control parts of the driver and utilizing the gpio-specific struct pinmux_ops callbacks. This solution looks like software configuration disguised as hardware configuration. Yours, Linus Walleij
在 2025-04-24星期四的 10:51 +0200，Linus Walleij写道： > On Thu, Apr 24, 2025 at 8:20 AM Icenowy Zheng <uwu@icenowy.me> wrote: > > > The JH7110 SoC could support internal GPI signals to be routed to > > not > > external GPIO but internal low/high levels. > > > > Add two macros, PAD_INTERNAL_LOW and PAD_INTERNAL_HIGH, as two > > virtual > > "pads" to represent internal GPI sources with fixed low/high > > levels. > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > As per my other reply in the previous post, I think this should be > handled internal in the kernel instead using a tighter integration > between > the GPIO and pin control parts of the driver and utilizing the > gpio-specific struct pinmux_ops callbacks. Well I cannot understand this -- these signals are not GPIOs, totally not related to the GPIO subsystem (because they're only pinmux, not related to GPIOs). This is described in my previous mail. The pin mux of JH7110 strictly route its inputs to its outputs. For signals from other SoC blocks (to external pins), the registers define how OUT/OEn of IO buffers *are driven by* the signals; however for signals to other SoC blocks (from external pins), the registers define how IN of IO buffers *drive* the signals. (This follows the generic signal-driving rule that one signal can drive multiple signals but cannot be multi-driven). In addition the situation I am trying to handle here is an addition to the latter part of the previous paragraph -- in addition to 64 inputs corresponding to 64 GPIOs, two extra inputs, one always 0 and another always 1 are available to the pin controller for driving other SoC blocks' input (as output of pin controller). In fact this is why there is ` + 2` when calculating ival in jh7110_set_gpiomux() -- the first two possible values are for always 0 and always 1, 3 represents the IN of GPIO0, etc. > > This solution looks like software configuration disguised as hardware > configuration. Well this solution handles these internal wires in the same way as signals from external GPIOs, excepting specifying special GPIO numbers. If you are against the principle, maybe the current already-included GPIOMUX system of the StarFive pinctrl is to be blamed instead of my small extension to it. I must admit that the current GPIOMUX system isn't a faithful representation of the hardware because it's a pad-centric setup instead of a register-field-centric one, which isn't very natural for input signals. However configurating the mux in such a way is against people reading, and we're not able to break the system because it's already there. Well in the situation that one GPIO used as input drives multiple internal signals the pinmux looks a little confusing too, e.g. the I2S clock situation I mentioned in my reply in the previous revision of the patchset. > > Yours, > Linus Walleij
On Thu, Apr 24, 2025 at 11:38 AM Icenowy Zheng <uwu@icenowy.me> wrote: > 在 2025-04-24星期四的 10:51 +0200，Linus Walleij写道： > > On Thu, Apr 24, 2025 at 8:20 AM Icenowy Zheng <uwu@icenowy.me> wrote: > > > > > The JH7110 SoC could support internal GPI signals to be routed to > > > not > > > external GPIO but internal low/high levels. > > > > > > Add two macros, PAD_INTERNAL_LOW and PAD_INTERNAL_HIGH, as two > > > virtual > > > "pads" to represent internal GPI sources with fixed low/high > > > levels. > > > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > > > As per my other reply in the previous post, I think this should be > > handled internal in the kernel instead using a tighter integration > > between > > the GPIO and pin control parts of the driver and utilizing the > > gpio-specific struct pinmux_ops callbacks. > > Well I cannot understand this -- these signals are not GPIOs, totally > not related to the GPIO subsystem (because they're only pinmux, not > related to GPIOs). This is described in my previous mail. OK sorry if I'm a bit dumb at times :( I guess I was falling into the common confusion of "something named general purpose" such as your GPI and GPO registers, is also related to GPIO which it is not, at least not always. > The pin mux of JH7110 strictly route its inputs to its outputs. For > signals from other SoC blocks (to external pins), the registers define > how OUT/OEn of IO buffers *are driven by* the signals; however for > signals to other SoC blocks (from external pins), the registers define > how IN of IO buffers *drive* the signals. (This follows the generic > signal-driving rule that one signal can drive multiple signals but > cannot be multi-driven). > > In addition the situation I am trying to handle here is an addition to > the latter part of the previous paragraph -- in addition to 64 inputs > corresponding to 64 GPIOs, two extra inputs, one always 0 and another > always 1 are available to the pin controller for driving other SoC > blocks' input (as output of pin controller). OK ... maybe I get it now. > > This solution looks like software configuration disguised as hardware > > configuration. > > Well this solution handles these internal wires in the same way as > signals from external GPIOs, excepting specifying special GPIO numbers. > If you are against the principle, maybe the current already-included > GPIOMUX system of the StarFive pinctrl is to be blamed instead of my > small extension to it. > > I must admit that the current GPIOMUX system isn't a faithful > representation of the hardware because it's a pad-centric setup instead > of a register-field-centric one, which isn't very natural for input > signals. However configurating the mux in such a way is against people > reading, and we're not able to break the system because it's already > there. > > Well in the situation that one GPIO used as input drives multiple > internal signals the pinmux looks a little confusing too, e.g. the I2S > clock situation I mentioned in my reply in the previous revision of the > patchset. I guess what rubs me the wrong way is why the external users (devices, device drivers or even pin hogs) cannot trigger the chain of events leading to this configuration, instead of different "magic" configurations that are just set up in the pin controller itself. But if you are positively convinced that there is no other way, I guess I have to live with it. Yours, Linus Walleij
在 2025-04-24星期四的 01:15 -0700，E Shattow写道： > On 4/23/25 23:20, Icenowy Zheng wrote: > > The JH7110 SoC could support internal GPI signals to be routed to > > not > > external GPIO but internal low/high levels. > > > > Add two macros, PAD_INTERNAL_LOW and PAD_INTERNAL_HIGH, as two > > virtual > > "pads" to represent internal GPI sources with fixed low/high > > levels. > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > --- > > include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h > > b/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h > > index 3865f01396395..3cca874b2bef7 100644 > > --- a/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h > > +++ b/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h > > @@ -126,6 +126,10 @@ > > #define PAD_GMAC0_TXEN 18 > > #define PAD_GMAC0_TXC 19 > > > > +/* virtual pins for forcing GPI */ > > +#define PAD_INTERNAL_LOW 254 > > +#define PAD_INTERNAL_HIGH 255 > > + > > #define GPOUT_LOW 0 > > #define GPOUT_HIGH 1 > > > > Asking about the choice of 255 and 254 values for virtual high/low > pins, > here. There's not much result when grep Linux source for 'virtual > pin' > to compare with. Are these the best values for this approach? These two values are picked because the following reasons: - The pin field has 8 bits (see the comments of jh7110_pinmux_din() in pinctrl-starfive-jh7110.c) - We are already using values 0 and 1 for GPIO0/GPIO1 If we're designing from scratch, it's possible to have another practice by using 0 and 1 for internal low/high and 2 for gpio0 so on. > > What happens when devicetree has in it to route PAD_INTERNAL_LOW to > PAD_INTERNAL_HIGH and other unlikely combinations? Or a devicetree > blob > with this computed value is paired to Linux kernel that does not have > the code to handle these virtual pins, for compatibility concern? I think it's not supported for newer DTs to be compatible with old kernels, but I analyzed the code, a read-out-of-bound could happen in jh7110_set_function() in pinctrl-starfive-jh7110-sys.c . Well this is unfortunate, but we can do few things to fix old kernels -- we can fix the problem in newer kernels. And even picking other values cannot make things better... > > Do we know yet if JH8100 will share some of this design? We don't know yet whether JH8100 can exist. > > -E