lkml.org 
[lkml]   [2021]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 0/4] add device drivers for Siemens Industrial PCs
Am Fri, 26 Nov 2021 08:02:11 -0800
schrieb Guenter Roeck <linux@roeck-us.net>:

> On 11/26/21 7:34 AM, Henning Schild wrote:
> > Hi all,
> >
> > in p3 not too much was left open so i hope this might be the last
> > and might be quick.
> >
> > The two points that have been open where:
> > 1 wish to split wdt driver into two
> > 2 wish to use pinctrl for LEDs/WDT
> >
> > 1 was ignored for now. We can split later when we add more devices.
> > It remains unclear if splitting is the way to go when more devices
> > come.
>
> The code is already quite messy, in part because memory regions are
> declared locally and not passed through the parent device as they
> should.

Oh is it, can you please be more precise when being so "harsh". The
parent driver is just a detector which devices in which variant are
available on a given box. It does not drive and should not claim
resources i guess.

> I don't see how splitting the driver into multiple drivers
> would improve the situation.

In fact that already is a split i would say.

> The platform code claims to be inspired by the lpc_ich driver.

Only the P2SB bit, getting the memory base of the pinctrl memory.

> Maybe it should take a real example from that
> and pass version or variant specific details through platform data
> instead of maintaining it in the watchdog driver.

It does that ... or i misunderstand you. The driver from p1 can find
these three watchdog types
SIMATIC_IPC_DEVICE_NONE
SIMATIC_IPC_DEVICE_227E
SIMATIC_IPC_DEVICE_427E

Which one to use is determined in the platform driver that will
platform_device_register_data(NULL, KBUILD_MODNAME "_wdt" ...
&platform_data)

So we tell the watchdog driver about a watchdog and tell it which one
...

If we ever split we could
register(KBUILD_MODNAME "_wdt_227e"
or
register(KBUILD_MODNAME "_wdt_427e"

with no platform data needed anymore.

But as i said, i do not want to split just yet. Just want to defend
that code when someone calls it "pretty messy" so late in the process.

Henning

> Guenter

\
 
 \ /
  Last update: 2021-11-26 18:31    [W:0.287 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site