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

[Henning Schild]

Thanks Andy,

Am Thu, 4 Mar 2021 12:19:44 +0200
schrieb Andy Shevchenko <andy.shevchenko@xxxxxxxxx>:

> On Thu, Mar 4, 2021 at 9:29 AM Henning Schild
> <henning.schild@xxxxxxxxxxx> wrote:
> 
> > This series adds support for watchdogs and leds of several x86
> > devices from Siemens.
> >
> > It is structured with a platform driver that mainly does
> > identification of the machines. It might trigger loading of the
> > actual device drivers by attaching devices to the platform bus.
> >
> > The identification is vendor specific, parsing a special binary DMI
> > entry. The implementation of that platform identification is
> > applied on pmc_atom clock quirks in the final patch.
> >
> > It is all structured in a way that we can easily add more devices
> > and more platform drivers later. Internally we have some more code
> > for hardware monitoring, more leds, watchdogs etc. This will follow
> > some day.
> >
> > But the idea here is to share early, and hopefully not fail early.  
> 
> I have given a few comments here and there, so please check the entire
> series and address them in _all_ similar locations. As I have noticed,
> I have different approach about P2SB code, I have to give the series a
> dust and see if you can utilize it.

You did find some things that others found as well. SPDX vs blabla,
header ordering, some other style.
Some things are already done and will be in v2.

Other findings are new, and we will look into them. The only thing that
did stick out is P2SB, also was a point in internal pre-review.
Let us see what you have, i can include a patch of yours into the q.
But i am kind of afraid once it is there, several existing users should
be touched with it, and this series would come later. Or this series
comes first and you come later and clean up our "mess". Not sure i
would want to take over all P2SB unhiders, but with you on board it
will work.

> I would like to be Cc'ed on the next version.

Sure thing.

regards,
Henning


>