Re: [PATCH v4 0/4] add device drivers for Siemens Industrial PCs

From: Guenter Roeck <linux@roeck-us.net>
To: Henning Schild <henning.schild@siemens.com>,
	linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
	platform-driver-x86@vger.kernel.org,
	linux-watchdog@vger.kernel.org
Cc: Srikanth Krishnakar <skrishnakar@gmail.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Gerd Haeussler <gerd.haeussler.ext@siemens.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Mark Gross <mgross@linux.intel.com>,
	Hans de Goede <hdegoede@redhat.com>, Pavel Machek <pavel@ucw.cz>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Enrico Weigelt <lkml@metux.net>
Subject: Re: [PATCH v4 0/4] add device drivers for Siemens Industrial PCs
Date: Fri, 26 Nov 2021 08:02:11 -0800	[thread overview]
Message-ID: <a1a6c43d-401c-19f1-9773-696402974f6b@roeck-us.net> (raw)
In-Reply-To: <20211126163410.2806de5f@md1za8fc.ad001.siemens.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.
I don't see how splitting the driver into multiple drivers would improve
the situation. The platform code claims to be inspired by the lpc_ich
driver. 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.

Guenter