2019-12-19 08:41:02

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] serdev: Don't claim unsupported serial devices

On Wed, Dec 18, 2019 at 9:56 AM Johan Hovold <[email protected]> wrote:
>
> On Wed, Dec 18, 2019 at 03:56:46PM +0900, Punit Agrawal wrote:
> > Serdev sub-system claims all serial devices that are not already
> > enumerated. As a result, no device node is created for serial port on
> > certain boards such as the Apollo Lake based UP2. This has the
> > unintended consequence of not being able to raise the login prompt via
> > serial connection.
> >
> > Introduce a blacklist to reject devices that should not be treated as
> > a serdev device. Add the Intel HS UART peripheral ids to the blacklist
> > to bring back serial port on SoCs carrying them.
> >
> > Cc: [email protected]
> > Signed-off-by: Punit Agrawal <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Johan Hovold <[email protected]>
> > Cc: Hans de Goede <[email protected]>
> > ---
> >
> > Hi,
> >
> > The patch has been updated based on feedback recieved on the RFC[0].
> >
> > Please consider merging if there are no objections.
>
> Rafael, I vaguely remember you arguing for a white list when we
> discussed this at some conference. Do you have any objections to the
> blacklist approach taken here?

As a rule, I prefer whitelisting, because it only enables the feature
for systems where it has been tested and confirmed to work.

However, if you are convinced that in this particular case the feature
should work on the vast majority of systems with a few possible
exceptions, blacklisting is fine too.

It all depends on what the majority is, at least in principle.


2019-12-19 08:53:22

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH] serdev: Don't claim unsupported serial devices

On Thu, Dec 19, 2019 at 09:39:57AM +0100, Rafael J. Wysocki wrote:
> On Wed, Dec 18, 2019 at 9:56 AM Johan Hovold <[email protected]> wrote:
> >
> > On Wed, Dec 18, 2019 at 03:56:46PM +0900, Punit Agrawal wrote:
> > > Serdev sub-system claims all serial devices that are not already
> > > enumerated. As a result, no device node is created for serial port on
> > > certain boards such as the Apollo Lake based UP2. This has the
> > > unintended consequence of not being able to raise the login prompt via
> > > serial connection.
> > >
> > > Introduce a blacklist to reject devices that should not be treated as
> > > a serdev device. Add the Intel HS UART peripheral ids to the blacklist
> > > to bring back serial port on SoCs carrying them.
> > >
> > > Cc: [email protected]
> > > Signed-off-by: Punit Agrawal <[email protected]>
> > > Cc: Rob Herring <[email protected]>
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Johan Hovold <[email protected]>
> > > Cc: Hans de Goede <[email protected]>
> > > ---
> > >
> > > Hi,
> > >
> > > The patch has been updated based on feedback recieved on the RFC[0].
> > >
> > > Please consider merging if there are no objections.
> >
> > Rafael, I vaguely remember you arguing for a white list when we
> > discussed this at some conference. Do you have any objections to the
> > blacklist approach taken here?
>
> As a rule, I prefer whitelisting, because it only enables the feature
> for systems where it has been tested and confirmed to work.
>
> However, if you are convinced that in this particular case the feature
> should work on the vast majority of systems with a few possible
> exceptions, blacklisting is fine too.
>
> It all depends on what the majority is, at least in principle.

Ok, thanks. I don't have a preference either way in this case simply
because I don't know the distribution you refer to.

But if Hans thinks blacklisting is the way to go then let's do that. We
haven't had that many reports about this, but if that were to change
down the line, I guess we can always switch to whitelisting.

Punit, feel free to add my

Acked-by: Johan Hovold <[email protected]>

after addressing the review comments you've gotten so far.

Johan

2019-12-19 08:59:23

by Punit Agrawal

[permalink] [raw]
Subject: Re: [PATCH] serdev: Don't claim unsupported serial devices

Johan Hovold <[email protected]> writes:

> On Thu, Dec 19, 2019 at 09:39:57AM +0100, Rafael J. Wysocki wrote:
>> On Wed, Dec 18, 2019 at 9:56 AM Johan Hovold <[email protected]> wrote:
>> >
>> > On Wed, Dec 18, 2019 at 03:56:46PM +0900, Punit Agrawal wrote:
>> > > Serdev sub-system claims all serial devices that are not already
>> > > enumerated. As a result, no device node is created for serial port on
>> > > certain boards such as the Apollo Lake based UP2. This has the
>> > > unintended consequence of not being able to raise the login prompt via
>> > > serial connection.
>> > >
>> > > Introduce a blacklist to reject devices that should not be treated as
>> > > a serdev device. Add the Intel HS UART peripheral ids to the blacklist
>> > > to bring back serial port on SoCs carrying them.
>> > >
>> > > Cc: [email protected]
>> > > Signed-off-by: Punit Agrawal <[email protected]>
>> > > Cc: Rob Herring <[email protected]>
>> > > Cc: Greg Kroah-Hartman <[email protected]>
>> > > Cc: Johan Hovold <[email protected]>
>> > > Cc: Hans de Goede <[email protected]>
>> > > ---
>> > >
>> > > Hi,
>> > >
>> > > The patch has been updated based on feedback recieved on the RFC[0].
>> > >
>> > > Please consider merging if there are no objections.
>> >
>> > Rafael, I vaguely remember you arguing for a white list when we
>> > discussed this at some conference. Do you have any objections to the
>> > blacklist approach taken here?
>>
>> As a rule, I prefer whitelisting, because it only enables the feature
>> for systems where it has been tested and confirmed to work.
>>
>> However, if you are convinced that in this particular case the feature
>> should work on the vast majority of systems with a few possible
>> exceptions, blacklisting is fine too.
>>
>> It all depends on what the majority is, at least in principle.
>
> Ok, thanks. I don't have a preference either way in this case simply
> because I don't know the distribution you refer to.
>
> But if Hans thinks blacklisting is the way to go then let's do that. We
> haven't had that many reports about this, but if that were to change
> down the line, I guess we can always switch to whitelisting.
>
> Punit, feel free to add my
>
> Acked-by: Johan Hovold <[email protected]>
>
> after addressing the review comments you've gotten so far.

Thanks Johan.

I will post a new version with the updates and acks.