The MacBookPro16,2 has a UART controller with pci id 8086:38a8. Added
this device to the intel_lpss_pci driver. This allows access to its
Bluetooth controller.
The name of this device is not known, however as it appears to only be
found on the one model (MacBookPro16,2), labelling it with that
model's name should be sufficient to identify it. (Unless some folks
from Intel know what to call it and what its section should be called.)
Records here indicate that this device is only present on the one model:
https://linux-hardware.org/?id=pci:8086-38a8-8086-7270
Signed-off-by: Orlando Chamberlain <[email protected]>
---
drivers/mfd/intel-lpss-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index c54d19fb184c..e02bb7026520 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
+ /* MacBookPro16,2 */
+ { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
/* TGL-H */
{ PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
{ PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
--
2.33.0
On Fri, Oct 01, 2021 at 08:51:46AM +0000, Orlando Chamberlain wrote:
Thanks for your contribution! My comments below.
> The MacBookPro16,2 has a UART controller with pci id 8086:38a8. Added
PCI ID
> this device to the intel_lpss_pci driver. This allows access to its
> Bluetooth controller.
> The name of this device is not known, however as it appears to only be
> found on the one model (MacBookPro16,2), labelling it with that
> model's name should be sufficient to identify it. (Unless some folks
> from Intel know what to call it and what its section should be called.)
No need to put this paragraph into commit message (see below).
> Records here indicate that this device is only present on the one model:
> https://linux-hardware.org/?id=pci:8086-38a8-8086-7270
>
> Signed-off-by: Orlando Chamberlain <[email protected]>
> ---
> drivers/mfd/intel-lpss-pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index c54d19fb184c..e02bb7026520 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
> { PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
> { PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
> { PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
> + /* MacBookPro16,2 */
Use
/* ICL-N */
here.
> + { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
> /* TGL-H */
> { PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
> { PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
--
With Best Regards,
Andy Shevchenko
Added 8086:38a8 to the intel_lpss_pci driver. It is a UART controler
present on the MacBookPro16,2.
Signed-off-by: Orlando Chamberlain <[email protected]>
---
v1->v2: Use "ICL-N" and shorten commit message.
drivers/mfd/intel-lpss-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index c54d19fb184c..a872b4485eac 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
+ /* ICL-N */
+ { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
/* TGL-H */
{ PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
{ PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
--
2.33.0
On Fri, Oct 01, 2021 at 07:14:12PM +0100, Lee Jones wrote:
> On Fri, 01 Oct 2021, Andy Shevchenko wrote:
> > On Fri, Oct 01, 2021 at 08:51:46AM +0000, Orlando Chamberlain wrote:
...
> > > + /* MacBookPro16,2 */
> >
> > Use
> >
> > /* ICL-N */
> >
> > here.
>
> Why is that?
It's a standard way of representing PCH/SoC in the comments in this driver:
- MacBookPro16,2 is not a name of the PCH/SoC
- MacBookPro16,2 is possible not the solely hardware of this chip
Why not to mention this name in the commit message?
--
With Best Regards,
Andy Shevchenko
On Fri, 01 Oct 2021, Andy Shevchenko wrote:
> On Fri, Oct 01, 2021 at 08:51:46AM +0000, Orlando Chamberlain wrote:
>
> Thanks for your contribution! My comments below.
>
> > The MacBookPro16,2 has a UART controller with pci id 8086:38a8. Added
>
> PCI ID
>
> > this device to the intel_lpss_pci driver. This allows access to its
> > Bluetooth controller.
>
> > The name of this device is not known, however as it appears to only be
> > found on the one model (MacBookPro16,2), labelling it with that
> > model's name should be sufficient to identify it. (Unless some folks
> > from Intel know what to call it and what its section should be called.)
>
> No need to put this paragraph into commit message (see below).
>
> > Records here indicate that this device is only present on the one model:
> > https://linux-hardware.org/?id=pci:8086-38a8-8086-7270
> >
> > Signed-off-by: Orlando Chamberlain <[email protected]>
> > ---
> > drivers/mfd/intel-lpss-pci.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> > index c54d19fb184c..e02bb7026520 100644
> > --- a/drivers/mfd/intel-lpss-pci.c
> > +++ b/drivers/mfd/intel-lpss-pci.c
> > @@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
> > { PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
> > { PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
> > { PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
>
> > + /* MacBookPro16,2 */
>
> Use
>
> /* ICL-N */
>
> here.
Why is that?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
PCH-N UART controler present on the MacBookPro16,2.
Signed-off-by: Orlando Chamberlain <[email protected]>
---
v2->v3: Mention "ICL-N" in commit message.
drivers/mfd/intel-lpss-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index c54d19fb184c..a872b4485eac 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
+ /* ICL-N */
+ { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
/* TGL-H */
{ PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
{ PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
--
2.33.0
On Sat, 02 Oct 2021 21:04:30 +1000
"Andy Shevchenko" <[email protected]> wrote:
> You ignoring my Reviewed-by tag. Any reason why?
Sorry, I'm new to this and assumed it needed to be reviewed again after
any changes (including to the commit message).
> Besides that you are posting patches as continuation of the thread.
> It may be problematic for some tools, like `b4`, although I dunno if
> Lee is using such tools.
I'll make a v4 with the reviewed-by line. I'll make it a reply to the
first version of the patch, and hopefully that'll be the first place
tools like `b4` look.
> Reviewed-by: Andy Shevchenko <[email protected]>
>
> > Signed-off-by: Orlando Chamberlain <[email protected]>
> > ---
> > v2->v3: Mention "ICL-N" in commit message.
> > drivers/mfd/intel-lpss-pci.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/mfd/intel-lpss-pci.c
> > b/drivers/mfd/intel-lpss-pci.c index c54d19fb184c..a872b4485eac
> > 100644 --- a/drivers/mfd/intel-lpss-pci.c
> > +++ b/drivers/mfd/intel-lpss-pci.c
> > @@ -253,6 +253,8 @@ static const struct pci_device_id
> > intel_lpss_pci_ids[] = { { PCI_VDEVICE(INTEL, 0x34ea),
> > (kernel_ulong_t)&bxt_i2c_info }, { PCI_VDEVICE(INTEL, 0x34eb),
> > (kernel_ulong_t)&bxt_i2c_info }, { PCI_VDEVICE(INTEL, 0x34fb),
> > (kernel_ulong_t)&spt_info },
> > + /* ICL-N */
> > + { PCI_VDEVICE(INTEL, 0x38a8),
> > (kernel_ulong_t)&bxt_uart_info }, /* TGL-H */
> > { PCI_VDEVICE(INTEL, 0x43a7),
> > (kernel_ulong_t)&bxt_uart_info }, { PCI_VDEVICE(INTEL, 0x43a8),
> > (kernel_ulong_t)&bxt_uart_info }, --
> > 2.33.0
> >
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
--
On Sat, Oct 02, 2021 at 03:31:07AM +0000, Orlando Chamberlain wrote:
> Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
> PCH-N UART controler present on the MacBookPro16,2.
You ignoring my Reviewed-by tag. Any reason why?
Besides that you are posting patches as continuation of the thread. It may be
problematic for some tools, like `b4`, although I dunno if Lee is using such
tools.
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Orlando Chamberlain <[email protected]>
> ---
> v2->v3: Mention "ICL-N" in commit message.
> drivers/mfd/intel-lpss-pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index c54d19fb184c..a872b4485eac 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
> { PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
> { PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
> { PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
> + /* ICL-N */
> + { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
> /* TGL-H */
> { PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
> { PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
> --
> 2.33.0
>
>
--
With Best Regards,
Andy Shevchenko
Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
PCH-N UART controler present on the MacBookPro16,2.
Signed-off-by: Orlando Chamberlain <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
v3->v4: reviewed-by line
drivers/mfd/intel-lpss-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index c54d19fb184c..a872b4485eac 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -253,6 +253,8 @@ static const struct pci_device_id intel_lpss_pci_ids[] = {
{ PCI_VDEVICE(INTEL, 0x34ea), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34eb), (kernel_ulong_t)&bxt_i2c_info },
{ PCI_VDEVICE(INTEL, 0x34fb), (kernel_ulong_t)&spt_info },
+ /* ICL-N */
+ { PCI_VDEVICE(INTEL, 0x38a8), (kernel_ulong_t)&bxt_uart_info },
/* TGL-H */
{ PCI_VDEVICE(INTEL, 0x43a7), (kernel_ulong_t)&bxt_uart_info },
{ PCI_VDEVICE(INTEL, 0x43a8), (kernel_ulong_t)&bxt_uart_info },
--
2.33.0
On Sat, Oct 02, 2021 at 11:12:26AM +0000, Orlando Chamberlain wrote:
> On Sat, 02 Oct 2021 21:04:30 +1000
> "Andy Shevchenko" <[email protected]> wrote:
...
> > Besides that you are posting patches as continuation of the thread.
> > It may be problematic for some tools, like `b4`, although I dunno if
> > Lee is using such tools.
>
> I'll make a v4 with the reviewed-by line. I'll make it a reply to the
> first version of the patch, and hopefully that'll be the first place
> tools like `b4` look.
Just make sure that you have --thread _disabled_ by default.
Linking to an (existing) thread, to whatever the point in it,
is still problematic.
(I dunno if you need to send a v5 because of that, I hope it's not needed)
--
With Best Regards,
Andy Shevchenko
On Sat, 02 Oct 2021, Orlando Chamberlain wrote:
> On Sat, 02 Oct 2021 21:04:30 +1000
> "Andy Shevchenko" <[email protected]> wrote:
>
> > You ignoring my Reviewed-by tag. Any reason why?
>
> Sorry, I'm new to this and assumed it needed to be reviewed again after
> any changes (including to the commit message).
>
> > Besides that you are posting patches as continuation of the thread.
> > It may be problematic for some tools, like `b4`, although I dunno if
> > Lee is using such tools.
>
> I'll make a v4 with the reviewed-by line. I'll make it a reply to the
> first version of the patch, and hopefully that'll be the first place
> tools like `b4` look.
You only need to add the *-by lines to *new* submissions.
Please don't submit new versions just to add *-by lines.
`b4` doesn't support replied-to patch-sets, so even replying-to the
very first submission/version won't help here. Submissions need to be
completely separate.
FWIW: As it happens, I do use `b4`, but I don't use the auto-apply-set
functionality. I either apply patches individually or select (tag)
the ones I wish to apply, so I do not suffer from this problem like
some others do.
> > Reviewed-by: Andy Shevchenko <[email protected]>
> >
> > > Signed-off-by: Orlando Chamberlain <[email protected]>
> > > ---
> > > v2->v3: Mention "ICL-N" in commit message.
> > > drivers/mfd/intel-lpss-pci.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/mfd/intel-lpss-pci.c
> > > b/drivers/mfd/intel-lpss-pci.c index c54d19fb184c..a872b4485eac
> > > 100644 --- a/drivers/mfd/intel-lpss-pci.c
> > > +++ b/drivers/mfd/intel-lpss-pci.c
> > > @@ -253,6 +253,8 @@ static const struct pci_device_id
> > > intel_lpss_pci_ids[] = { { PCI_VDEVICE(INTEL, 0x34ea),
> > > (kernel_ulong_t)&bxt_i2c_info }, { PCI_VDEVICE(INTEL, 0x34eb),
> > > (kernel_ulong_t)&bxt_i2c_info }, { PCI_VDEVICE(INTEL, 0x34fb),
> > > (kernel_ulong_t)&spt_info },
> > > + /* ICL-N */
> > > + { PCI_VDEVICE(INTEL, 0x38a8),
> > > (kernel_ulong_t)&bxt_uart_info }, /* TGL-H */
> > > { PCI_VDEVICE(INTEL, 0x43a7),
> > > (kernel_ulong_t)&bxt_uart_info }, { PCI_VDEVICE(INTEL, 0x43a8),
> > > (kernel_ulong_t)&bxt_uart_info }, --
> > > 2.33.0
> > >
> > >
> >
>
>
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, 01 Oct 2021, Andy Shevchenko wrote:
> On Fri, Oct 01, 2021 at 07:14:12PM +0100, Lee Jones wrote:
> > On Fri, 01 Oct 2021, Andy Shevchenko wrote:
> > > On Fri, Oct 01, 2021 at 08:51:46AM +0000, Orlando Chamberlain wrote:
>
> ...
>
> > > > + /* MacBookPro16,2 */
> > >
> > > Use
> > >
> > > /* ICL-N */
> > >
> > > here.
> >
> > Why is that?
>
> It's a standard way of representing PCH/SoC in the comments in this driver:
> - MacBookPro16,2 is not a name of the PCH/SoC
> - MacBookPro16,2 is possible not the solely hardware of this chip
Okay, that's fine.
> Why not to mention this name in the commit message?
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 02 Oct 2021, Orlando Chamberlain wrote:
> Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
> PCH-N UART controler present on the MacBookPro16,2.
>
> Signed-off-by: Orlando Chamberlain <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> v3->v4: reviewed-by line
> drivers/mfd/intel-lpss-pci.c | 2 ++
> 1 file changed, 2 insertions(+)
Applied, thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Hi Lee
I request you to Backport this to 5.10 as well
Regards
Aditya
> On 04-Oct-2021, at 12:47 PM, Lee Jones <[email protected]> wrote:
>
> On Sat, 02 Oct 2021, Orlando Chamberlain wrote:
>
>> Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
>> PCH-N UART controler present on the MacBookPro16,2.
>>
>> Signed-off-by: Orlando Chamberlain <[email protected]>
>> Reviewed-by: Andy Shevchenko <[email protected]>
>> ---
>> v3->v4: reviewed-by line
>> drivers/mfd/intel-lpss-pci.c | 2 ++
>> 1 file changed, 2 insertions(+)
>
> Applied, thanks.
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog
>
On Mon, 04 Oct 2021, Aditya Garg wrote:
> Hi Lee
>
> I request you to Backport this to 5.10 as well
Please read:
Documentation/process/stable-kernel-rules.rst
If you think this patches meets the criteria you can submit it for
inclusion to the Stable kernel yourself using one of the documented
'Options'.
> > On 04-Oct-2021, at 12:47 PM, Lee Jones <[email protected]> wrote:
> >
> > On Sat, 02 Oct 2021, Orlando Chamberlain wrote:
> >
> >> Added 8086:38a8 to the intel_lpss_pci driver. It is an Intel Ice Lake
> >> PCH-N UART controler present on the MacBookPro16,2.
> >>
> >> Signed-off-by: Orlando Chamberlain <[email protected]>
> >> Reviewed-by: Andy Shevchenko <[email protected]>
> >> ---
> >> v3->v4: reviewed-by line
> >> drivers/mfd/intel-lpss-pci.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >
> > Applied, thanks.
> >
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog