Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] serdev: Don't claim unsupported serial devices
@ 2019-12-18  6:56 Punit Agrawal
  2019-12-18  8:10 ` Greg Kroah-Hartman
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Punit Agrawal @ 2019-12-18  6:56 UTC (permalink / raw)
  To: linux-serial
  Cc: Punit Agrawal, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Greg Kroah-Hartman,
	Johan Hovold, Hans de Goede

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: stable@vger.kernel.org
Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
Cc: Rob Herring <robh@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
---

Hi,

The patch has been updated based on feedback recieved on the RFC[0].

Please consider merging if there are no objections.

Thanks,
Punit

[0] https://www.spinics.net/lists/linux-serial/msg36646.html

 drivers/tty/serdev/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 226adeec2aed..0d64fb7d4f36 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
 	return AE_OK;
 }
 
+static const struct acpi_device_id serdev_blacklist_devices[] = {
+	{"INT3511", 0},
+	{"INT3512", 0},
+	{ },
+};
+
 static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
 					  void *data, void **return_value)
 {
@@ -675,6 +681,10 @@ static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
 	if (acpi_device_enumerated(adev))
 		return AE_OK;
 
+	/* Skip if black listed */
+	if (!acpi_match_device_ids(adev, serdev_blacklist_devices))
+		return AE_OK;
+
 	if (acpi_serdev_check_resources(ctrl, adev))
 		return AE_OK;
 
-- 
2.24.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  6:56 [PATCH] serdev: Don't claim unsupported serial devices Punit Agrawal
@ 2019-12-18  8:10 ` Greg Kroah-Hartman
  2019-12-18  8:22   ` Punit Agrawal
  2019-12-18  8:56 ` Johan Hovold
  2019-12-18 10:05 ` Hans de Goede
  2 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-18  8:10 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-serial, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Johan Hovold, Hans de Goede

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.

All ACPI serial devices, right?  Surely not all other types of serial
devices in the system.

And what do you mean by "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

"reject ACPI serial devices"

> a serdev device. Add the Intel HS UART peripheral ids to the blacklist
> to bring back serial port on SoCs carrying them.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> ---
> 
> Hi,
> 
> The patch has been updated based on feedback recieved on the RFC[0].
> 
> Please consider merging if there are no objections.
> 
> Thanks,
> Punit
> 
> [0] https://www.spinics.net/lists/linux-serial/msg36646.html
> 
>  drivers/tty/serdev/core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 226adeec2aed..0d64fb7d4f36 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>  	return AE_OK;
>  }
>  
> +static const struct acpi_device_id serdev_blacklist_devices[] = {

s/serdev_blacklist_devices/serdev_blacklist/acpi_devices/  ?

This is an acpi-specific thing, not a generic tty thing.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  8:10 ` Greg Kroah-Hartman
@ 2019-12-18  8:22   ` Punit Agrawal
  0 siblings, 0 replies; 11+ messages in thread
From: Punit Agrawal @ 2019-12-18  8:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-serial, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Johan Hovold, Hans de Goede

Hi Greg,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> 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.
>
> All ACPI serial devices, right?  Surely not all other types of serial
> devices in the system.

That's right - I'll tighten the wording in the commit log and the patch
below to be ACPI specific.

>
> And what do you mean by "not already enumerated"?

This is referring to check using "acpi_device_enumerated()" when walking
ACPI devices. But the intention was to convey uninitialised serial
devices exposed via ACPI.

I'll update this as well.

Thanks for the review.

Punit

>
>> 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
>
> "reject ACPI serial devices"
>
>> a serdev device. Add the Intel HS UART peripheral ids to the blacklist
>> to bring back serial port on SoCs carrying them.
>> 
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> ---
>> 
>> Hi,
>> 
>> The patch has been updated based on feedback recieved on the RFC[0].
>> 
>> Please consider merging if there are no objections.
>> 
>> Thanks,
>> Punit
>> 
>> [0] https://www.spinics.net/lists/linux-serial/msg36646.html
>> 
>>  drivers/tty/serdev/core.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 226adeec2aed..0d64fb7d4f36 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>>  	return AE_OK;
>>  }
>>  
>> +static const struct acpi_device_id serdev_blacklist_devices[] = {
>
> s/serdev_blacklist_devices/serdev_blacklist/acpi_devices/  ?
>
> This is an acpi-specific thing, not a generic tty thing.
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  6:56 [PATCH] serdev: Don't claim unsupported serial devices Punit Agrawal
  2019-12-18  8:10 ` Greg Kroah-Hartman
@ 2019-12-18  8:56 ` Johan Hovold
  2019-12-18  9:09   ` Punit Agrawal
  2019-12-19  8:39   ` Rafael J. Wysocki
  2019-12-18 10:05 ` Hans de Goede
  2 siblings, 2 replies; 11+ messages in thread
From: Johan Hovold @ 2019-12-18  8:56 UTC (permalink / raw)
  To: Punit Agrawal, Rafael J. Wysocki
  Cc: linux-serial, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Greg Kroah-Hartman,
	Johan Hovold, Hans de Goede

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: stable@vger.kernel.org
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> ---
> 
> 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?

> [0] https://www.spinics.net/lists/linux-serial/msg36646.html
> 
>  drivers/tty/serdev/core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 226adeec2aed..0d64fb7d4f36 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>  	return AE_OK;
>  }
>  
> +static const struct acpi_device_id serdev_blacklist_devices[] = {
> +	{"INT3511", 0},
> +	{"INT3512", 0},

Nit: Would you mind adding a space after { and before }?

> +	{ },
> +};

Johan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  8:56 ` Johan Hovold
@ 2019-12-18  9:09   ` Punit Agrawal
  2019-12-19  8:39   ` Rafael J. Wysocki
  1 sibling, 0 replies; 11+ messages in thread
From: Punit Agrawal @ 2019-12-18  9:09 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Rafael J. Wysocki, linux-serial, linux-acpi, linux-kernel,
	nobuhiro1.iwamatsu, shrirang.bagul, stable, Rob Herring,
	Greg Kroah-Hartman, Hans de Goede

Hi Johan,

Johan Hovold <johan@kernel.org> writes:

> 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: stable@vger.kernel.org
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> ---
>> 
>> 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?
>
>> [0] https://www.spinics.net/lists/linux-serial/msg36646.html
>> 
>>  drivers/tty/serdev/core.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
>> index 226adeec2aed..0d64fb7d4f36 100644
>> --- a/drivers/tty/serdev/core.c
>> +++ b/drivers/tty/serdev/core.c
>> @@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>>  	return AE_OK;
>>  }
>>  
>> +static const struct acpi_device_id serdev_blacklist_devices[] = {
>> +	{"INT3511", 0},
>> +	{"INT3512", 0},
>
> Nit: Would you mind adding a space after { and before }?

Sure - no problem. I'll include it in the next posting.

Thanks for having a look.

>
>> +	{ },
>> +};
>
> Johan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  6:56 [PATCH] serdev: Don't claim unsupported serial devices Punit Agrawal
  2019-12-18  8:10 ` Greg Kroah-Hartman
  2019-12-18  8:56 ` Johan Hovold
@ 2019-12-18 10:05 ` Hans de Goede
  2019-12-19  0:37   ` Punit Agrawal
  2 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2019-12-18 10:05 UTC (permalink / raw)
  To: Punit Agrawal, linux-serial
  Cc: linux-acpi, linux-kernel, nobuhiro1.iwamatsu, shrirang.bagul,
	stable, Rob Herring, Greg Kroah-Hartman, Johan Hovold

Hi,

On 18-12-2019 07:56, 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: stable@vger.kernel.org
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>

Thank you for addressing this long standing issue.

The basic approach here looks good to me, once the minor
comments from other reviewers are addressed you can add my:

Acked-by: Hans de Goede <hdegoede@redhat.com>

to the next version.

Regards,

Hans



> ---
> 
> Hi,
> 
> The patch has been updated based on feedback recieved on the RFC[0].
> 
> Please consider merging if there are no objections.
> 
> Thanks,
> Punit
> 
> [0] https://www.spinics.net/lists/linux-serial/msg36646.html
> 
>   drivers/tty/serdev/core.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 226adeec2aed..0d64fb7d4f36 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -663,6 +663,12 @@ static acpi_status acpi_serdev_register_device(struct serdev_controller *ctrl,
>   	return AE_OK;
>   }
>   
> +static const struct acpi_device_id serdev_blacklist_devices[] = {
> +	{"INT3511", 0},
> +	{"INT3512", 0},
> +	{ },
> +};
> +
>   static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
>   					  void *data, void **return_value)
>   {
> @@ -675,6 +681,10 @@ static acpi_status acpi_serdev_add_device(acpi_handle handle, u32 level,
>   	if (acpi_device_enumerated(adev))
>   		return AE_OK;
>   
> +	/* Skip if black listed */
> +	if (!acpi_match_device_ids(adev, serdev_blacklist_devices))
> +		return AE_OK;
> +
>   	if (acpi_serdev_check_resources(ctrl, adev))
>   		return AE_OK;
>   
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18 10:05 ` Hans de Goede
@ 2019-12-19  0:37   ` Punit Agrawal
  2019-12-19  8:30     ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Punit Agrawal @ 2019-12-19  0:37 UTC (permalink / raw)
  To: Hans de Goede
  Cc: linux-serial, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Greg Kroah-Hartman,
	Johan Hovold

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>
> On 18-12-2019 07:56, 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: stable@vger.kernel.org
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>
> Thank you for addressing this long standing issue.

I am surprised there hasn't been more people complaining! Maybe even on
x86 mainline isn't that widely used on development boards.

> The basic approach here looks good to me, once the minor
> comments from other reviewers are addressed you can add my:
>
> Acked-by: Hans de Goede <hdegoede@redhat.com>

Thanks!

[...]


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-19  0:37   ` Punit Agrawal
@ 2019-12-19  8:30     ` Hans de Goede
  0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2019-12-19  8:30 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-serial, linux-acpi, linux-kernel, nobuhiro1.iwamatsu,
	shrirang.bagul, stable, Rob Herring, Greg Kroah-Hartman,
	Johan Hovold

Hi,

On 19-12-2019 01:37, Punit Agrawal wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
> 
>> Hi,
>>
>> On 18-12-2019 07:56, 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: stable@vger.kernel.org
>>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Johan Hovold <johan@kernel.org>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>
>> Thank you for addressing this long standing issue.
> 
> I am surprised there hasn't been more people complaining! Maybe even on
> x86 mainline isn't that widely used on development boards.

I think it is also a case of there not being that manu x86 development
boards.

Regards,

Hans


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-18  8:56 ` Johan Hovold
  2019-12-18  9:09   ` Punit Agrawal
@ 2019-12-19  8:39   ` Rafael J. Wysocki
  2019-12-19  8:51     ` Johan Hovold
  1 sibling, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2019-12-19  8:39 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Punit Agrawal, Rafael J. Wysocki, linux-serial,
	ACPI Devel Maling List, Linux Kernel Mailing List,
	nobuhiro1.iwamatsu, shrirang.bagul, Stable, Rob Herring,
	Greg Kroah-Hartman, Hans de Goede

On Wed, Dec 18, 2019 at 9:56 AM Johan Hovold <johan@kernel.org> 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: stable@vger.kernel.org
> > Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Johan Hovold <johan@kernel.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > ---
> >
> > 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-19  8:39   ` Rafael J. Wysocki
@ 2019-12-19  8:51     ` Johan Hovold
  2019-12-19  8:58       ` Punit Agrawal
  0 siblings, 1 reply; 11+ messages in thread
From: Johan Hovold @ 2019-12-19  8:51 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Johan Hovold, Punit Agrawal, Rafael J. Wysocki, linux-serial,
	ACPI Devel Maling List, Linux Kernel Mailing List,
	nobuhiro1.iwamatsu, shrirang.bagul, Stable, Rob Herring,
	Greg Kroah-Hartman, Hans de Goede

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 <johan@kernel.org> 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: stable@vger.kernel.org
> > > Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Johan Hovold <johan@kernel.org>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >
> > > 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 <johan@kernel.org>

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

Johan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] serdev: Don't claim unsupported serial devices
  2019-12-19  8:51     ` Johan Hovold
@ 2019-12-19  8:58       ` Punit Agrawal
  0 siblings, 0 replies; 11+ messages in thread
From: Punit Agrawal @ 2019-12-19  8:58 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-serial,
	ACPI Devel Maling List, Linux Kernel Mailing List,
	nobuhiro1.iwamatsu, shrirang.bagul, Stable, Rob Herring,
	Greg Kroah-Hartman, Hans de Goede

Johan Hovold <johan@kernel.org> 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 <johan@kernel.org> 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: stable@vger.kernel.org
>> > > Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> > > Cc: Rob Herring <robh@kernel.org>
>> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > > Cc: Johan Hovold <johan@kernel.org>
>> > > Cc: Hans de Goede <hdegoede@redhat.com>
>> > > ---
>> > >
>> > > 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 <johan@kernel.org>
>
> after addressing the review comments you've gotten so far.

Thanks Johan.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18  6:56 [PATCH] serdev: Don't claim unsupported serial devices Punit Agrawal
2019-12-18  8:10 ` Greg Kroah-Hartman
2019-12-18  8:22   ` Punit Agrawal
2019-12-18  8:56 ` Johan Hovold
2019-12-18  9:09   ` Punit Agrawal
2019-12-19  8:39   ` Rafael J. Wysocki
2019-12-19  8:51     ` Johan Hovold
2019-12-19  8:58       ` Punit Agrawal
2019-12-18 10:05 ` Hans de Goede
2019-12-19  0:37   ` Punit Agrawal
2019-12-19  8:30     ` Hans de Goede

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git