On Tue, Mar 30, 2021 at 2:58 PM Henning Schild <henning.schild@xxxxxxxxxxx> wrote: > Am Tue, 30 Mar 2021 14:04:35 +0300 > schrieb Andy Shevchenko <andy.shevchenko@xxxxxxxxx>: > > On Mon, Mar 29, 2021 at 8:59 PM Henning Schild > > <henning.schild@xxxxxxxxxxx> wrote: > > > > > > This driver adds initial support for several devices from Siemens. > > > It is based on a platform driver introduced in an earlier commit. > > > > ... > > > > > +#define SIMATIC_IPC_LED_PORT_BASE 0x404E > > > > > +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > > > + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > > > + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > > > + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > > > + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > > > + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > > > + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > > > + { } > > > +}; > > > > > +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { > > > + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, > > > + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, > > > + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, > > > + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, > > > + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, > > > + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, > > > + { } > > > +}; > > > > It seems to me like poking GPIO controller registers directly. This > > is not good. The question still remains: Can we simply register a > > GPIO (pin control) driver and use an LED GPIO driver with an > > additional board file that instantiates it? > > I wrote about that in reply to the cover letter. My view is still that > it would be an abstraction with only one user, just causing work and > likely not ending up as generic as it might eventually have to be. > > The region is reserved, not sure what the problem with the "poking" is. > Maybe i do not understand all the benefits of such a split at this > point in time. At the moment i only see work with hardly any benefit, > not just work for me but also for maintainers. I sure do not mean to be > ignorant. Maybe you go into details and convince me or we wait for other > peoples opinions on how to proceed, maybe there is a second user that i > am not aware of? > Until i am convinced otherwise i will try to argue that a > single-user-abstraction is needless work/code, and should be done only > when actually needed. I have just read your messages (there is a cover letter and additional email which was sent lately). I would like to know what the CPU model number on that board is. Than we can continue to see what possibilities we have here. -- With Best Regards, Andy Shevchenko