Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1300788 > unrolled thread

Re: [PATCH 1/6] 8250/Kconfig: add config option CONFIG_SERIAL_8250_AMD

Started byBorislav Petkov <bp@alien8.de>
First post2016-01-04 15:50 +0100
Last post2016-01-11 08:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/6] 8250/Kconfig: add config option  CONFIG_SERIAL_8250_AMD Borislav Petkov <bp@alien8.de> - 2016-01-04 15:50 +0100
    RE: [PATCH 1/6] 8250/Kconfig: add config option  CONFIG_SERIAL_8250_AMD "Wang, Annie" <Annie.Wang@amd.com> - 2016-01-06 03:10 +0100
      Re: [PATCH 1/6] 8250/Kconfig: add config option  CONFIG_SERIAL_8250_AMD Borislav Petkov <bp@alien8.de> - 2016-01-06 11:50 +0100
        RE: [PATCH 1/6] 8250/Kconfig: add config option  CONFIG_SERIAL_8250_AMD "Wang, Annie" <Annie.Wang@amd.com> - 2016-01-11 08:30 +0100

#1300788 — Re: [PATCH 1/6] 8250/Kconfig: add config option CONFIG_SERIAL_8250_AMD

FromBorislav Petkov <bp@alien8.de>
Date2016-01-04 15:50 +0100
SubjectRe: [PATCH 1/6] 8250/Kconfig: add config option CONFIG_SERIAL_8250_AMD
Message-ID<qNeCm-4IB-17@gated-at.bofh.it>
On Mon, Jan 04, 2016 at 01:31:36PM +0800, Wang Hongcheng wrote:
> Add config option  CONFIG_SERIAL_8250_AMD in use of AMD carrizo.
> Because carrizo's UART DMA device is an amba device, it selects
> ARM_AMBA option. Anything uses amba devices must select ARM_AMBA.
> 
> Signed-off-by: Wang Hongcheng <annie.wang@amd.com>
> ---
>  drivers/tty/serial/8250/Kconfig | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 6412f14..c9ebc31 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -378,3 +378,11 @@ config SERIAL_8250_MID
>  	  Selecting this option will enable handling of the extra features
>  	  present on the UART found on Intel Medfield SOC and various other
>  	  Intel platforms.
> +
> +config SERIAL_8250_AMD
> +	bool "AMD carrizo serial port support"
> +	depends on SERIAL_8250
> +	select ARM_AMBA
> +	help
> +	  If you have a Family 15h, models 0x60-0x6F based board and want to
> +	  use the serial port, say Y to this option. If unsure, say N.

Hmm, so you're adding this config option here only to have
acpi_apd_setup_quirks() defined in an already AMD-specific compilation
unit drivers/acpi/acpi_apd.c.

So why not make drivers/acpi/acpi_apd.c depend on CPU_SUP_AMD
and this way it is automatically enabled on AMD and then check
family/model/stepping when assigning that

+       .post_setup = acpi_apd_setup_quirks,

thing?

You need it only on F15h, models 0x60.. only so you don't really need
the config option when you can find out on what hardware you're running
without the user having to configure the kernel.

Hmmm?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1302337

From"Wang, Annie" <Annie.Wang@amd.com>
Date2016-01-06 03:10 +0100
Message-ID<qNLHX-36E-9@gated-at.bofh.it>
In reply to#1300788
SGkgQm9yaXMsDQoNCj4tLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPkZyb206IEJvcmlzbGF2
IFBldGtvdiBbbWFpbHRvOmJwQGFsaWVuOC5kZV0NCj5TZW50OiBNb25kYXksIEphbnVhcnkgMDQs
IDIwMTYgMTA6NDEgUE0NCj5UbzogV2FuZywgQW5uaWUNCj5DYzogQW5keSBTaGV2Y2hlbmtvOyBW
aW5vZCBLb3VsOyBNaWthIFdlc3RlcmJlcmc7IEdyZWcgS3JvYWgtSGFydG1hbjsgUmFmYWVsDQo+
Si4gV3lzb2NraTsgbGludXgtYWNwaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2Vy
Lmtlcm5lbC5vcmc7IGxpbnV4LQ0KPnNlcmlhbEB2Z2VyLmtlcm5lbC5vcmc7IGRtYWVuZ2luZUB2
Z2VyLmtlcm5lbC5vcmc7IEh1YW5nLCBSYXk7IFdhbiwgVmluY2VudDsNCj5YdWUsIEtlbjsgUm9i
aW4gTXVycGh5OyBHcmFlbWUgR3JlZ29yeTsgTGksIFRvbnk7IFl1LCBYaWFuZ2xpYW5nDQo+U3Vi
amVjdDogUmU6IFtQQVRDSCAxLzZdIDgyNTAvS2NvbmZpZzogYWRkIGNvbmZpZyBvcHRpb24NCj5D
T05GSUdfU0VSSUFMXzgyNTBfQU1EDQo+DQo+T24gTW9uLCBKYW4gMDQsIDIwMTYgYXQgMDE6MzE6
MzZQTSArMDgwMCwgV2FuZyBIb25nY2hlbmcgd3JvdGU6DQo+PiBBZGQgY29uZmlnIG9wdGlvbiAg
Q09ORklHX1NFUklBTF84MjUwX0FNRCBpbiB1c2Ugb2YgQU1EIGNhcnJpem8uDQo+PiBCZWNhdXNl
IGNhcnJpem8ncyBVQVJUIERNQSBkZXZpY2UgaXMgYW4gYW1iYSBkZXZpY2UsIGl0IHNlbGVjdHMN
Cj4+IEFSTV9BTUJBIG9wdGlvbi4gQW55dGhpbmcgdXNlcyBhbWJhIGRldmljZXMgbXVzdCBzZWxl
Y3QgQVJNX0FNQkEuDQo+Pg0KPj4gU2lnbmVkLW9mZi1ieTogV2FuZyBIb25nY2hlbmcgPGFubmll
LndhbmdAYW1kLmNvbT4NCj4+IC0tLQ0KPj4gIGRyaXZlcnMvdHR5L3NlcmlhbC84MjUwL0tjb25m
aWcgfCA4ICsrKysrKysrDQo+PiAgMSBmaWxlIGNoYW5nZWQsIDggaW5zZXJ0aW9ucygrKQ0KPj4N
Cj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3R0eS9zZXJpYWwvODI1MC9LY29uZmlnDQo+PiBiL2Ry
aXZlcnMvdHR5L3NlcmlhbC84MjUwL0tjb25maWcgaW5kZXggNjQxMmYxNC4uYzllYmMzMSAxMDA2
NDQNCj4+IC0tLSBhL2RyaXZlcnMvdHR5L3NlcmlhbC84MjUwL0tjb25maWcNCj4+ICsrKyBiL2Ry
aXZlcnMvdHR5L3NlcmlhbC84MjUwL0tjb25maWcNCj4+IEBAIC0zNzgsMyArMzc4LDExIEBAIGNv
bmZpZyBTRVJJQUxfODI1MF9NSUQNCj4+ICAJICBTZWxlY3RpbmcgdGhpcyBvcHRpb24gd2lsbCBl
bmFibGUgaGFuZGxpbmcgb2YgdGhlIGV4dHJhIGZlYXR1cmVzDQo+PiAgCSAgcHJlc2VudCBvbiB0
aGUgVUFSVCBmb3VuZCBvbiBJbnRlbCBNZWRmaWVsZCBTT0MgYW5kIHZhcmlvdXMgb3RoZXINCj4+
ICAJICBJbnRlbCBwbGF0Zm9ybXMuDQo+PiArDQo+PiArY29uZmlnIFNFUklBTF84MjUwX0FNRA0K
Pj4gKwlib29sICJBTUQgY2Fycml6byBzZXJpYWwgcG9ydCBzdXBwb3J0Ig0KPj4gKwlkZXBlbmRz
IG9uIFNFUklBTF84MjUwDQo+PiArCXNlbGVjdCBBUk1fQU1CQQ0KPj4gKwloZWxwDQo+PiArCSAg
SWYgeW91IGhhdmUgYSBGYW1pbHkgMTVoLCBtb2RlbHMgMHg2MC0weDZGIGJhc2VkIGJvYXJkIGFu
ZCB3YW50IHRvDQo+PiArCSAgdXNlIHRoZSBzZXJpYWwgcG9ydCwgc2F5IFkgdG8gdGhpcyBvcHRp
b24uIElmIHVuc3VyZSwgc2F5IE4uDQo+DQo+SG1tLCBzbyB5b3UncmUgYWRkaW5nIHRoaXMgY29u
ZmlnIG9wdGlvbiBoZXJlIG9ubHkgdG8gaGF2ZQ0KPmFjcGlfYXBkX3NldHVwX3F1aXJrcygpIGRl
ZmluZWQgaW4gYW4gYWxyZWFkeSBBTUQtc3BlY2lmaWMgY29tcGlsYXRpb24gdW5pdA0KPmRyaXZl
cnMvYWNwaS9hY3BpX2FwZC5jLg0KPg0KDQpIb3cgYWJvdXQgSSBhZGQgc2VsZWN0IEFSTV9BTUJB
IGFuZCBTRVJJQUxfODI1MCBpbiBhcmNoL3g4Ni9LY29uZmlnPw0KDQpkaWZmIC0tZ2l0IGEvYXJj
aC94ODYvS2NvbmZpZyBiL2FyY2gveDg2L0tjb25maWcNCmluZGV4IGRiMzYyMmYuLjBmZTY2NTcg
MTAwNjQ0DQotLS0gYS9hcmNoL3g4Ni9LY29uZmlnDQorKysgYi9hcmNoL3g4Ni9LY29uZmlnDQpA
QCAtNTM3LDExICs1MzcsMTUgQEAgY29uZmlnIFg4Nl9BTURfUExBVEZPUk1fREVWSUNFDQogICAg
ICAgIGRlcGVuZHMgb24gQUNQSQ0KICAgICAgICBzZWxlY3QgQ09NTU9OX0NMSw0KICAgICAgICBz
ZWxlY3QgUElOQ1RSTA0KKyAgICAgICBzZWxlY3QgU0VSSUFMXzgyNTANCisgICAgICAgc2VsZWN0
IEFSTV9BTUJBDQogICAgICAgIC0tLWhlbHAtLS0NCiAgICAgICAgICBTZWxlY3QgdG8gaW50ZXJw
cmV0IEFNRCBzcGVjaWZpYyBBQ1BJIGRldmljZSB0byBwbGF0Zm9ybSBkZXZpY2UNCiAgICAgICAg
ICBzdWNoIGFzIEkyQywgVUFSVCwgR1BJTyBmb3VuZCBvbiBBTUQgQ2Fycml6byBhbmQgbGF0ZXIg
Y2hpcHNldHMuDQogICAgICAgICAgSTJDIGFuZCBVQVJUIGRlcGVuZCBvbiBDT01NT05fQ0xLIHRv
IHNldCBjbG9jay4gR1BJTyBkcml2ZXIgaXMNCi0gICAgICAgICBpbXBsZW1lbnRlZCB1bmRlciBQ
SU5DVFJMIHN1YnN5c3RlbS4NCisgICAgICAgICBpbXBsZW1lbnRlZCB1bmRlciBQSU5DVFJMIHN1
YnN5c3RlbS4gQ2Fycml6bydzIFVBUlQgaXMgaW1wbGVtZW50ZWQNCisgICAgICAgICB1bmRlciBT
RVJJQUxfODI1MC4gQ2Fycml6bydzIFVBUlQgRE1BIGRldmljZSBpcyBhbiBhbWJhIGRldmljZSwN
CisgICAgICAgICBpdCBzZWxlY3RzIEFSTV9BTUJBIG9wdGlvbi4NCg0KIGNvbmZpZyBJT1NGX01C
SQ0KICAgICAgICB0cmlzdGF0ZSAiSW50ZWwgU29DIElPU0YgU2lkZWJhbmQgc3VwcG9ydCBmb3Ig
U29DIHBsYXRmb3JtcyINCi0tDQoNCj5TbyB3aHkgbm90IG1ha2UgZHJpdmVycy9hY3BpL2FjcGlf
YXBkLmMgZGVwZW5kIG9uIENQVV9TVVBfQU1EIGFuZCB0aGlzIHdheQ0KPml0IGlzIGF1dG9tYXRp
Y2FsbHkgZW5hYmxlZCBvbiBBTUQgYW5kIHRoZW4gY2hlY2sgZmFtaWx5L21vZGVsL3N0ZXBwaW5n
IHdoZW4NCj5hc3NpZ25pbmcgdGhhdA0KPg0KPisgICAgICAgLnBvc3Rfc2V0dXAgPSBhY3BpX2Fw
ZF9zZXR1cF9xdWlya3MsDQo+DQo+dGhpbmc/DQo+DQo+WW91IG5lZWQgaXQgb25seSBvbiBGMTVo
LCBtb2RlbHMgMHg2MC4uIG9ubHkgc28geW91IGRvbid0IHJlYWxseSBuZWVkIHRoZSBjb25maWcN
Cj5vcHRpb24gd2hlbiB5b3UgY2FuIGZpbmQgb3V0IG9uIHdoYXQgaGFyZHdhcmUgeW91J3JlIHJ1
bm5pbmcgd2l0aG91dCB0aGUgdXNlcg0KPmhhdmluZyB0byBjb25maWd1cmUgdGhlIGtlcm5lbC4N
Cj4NCj5IbW1tPw0KDQpDUFVfU1VQX0FNRCBpcyBvbmx5IGNvbmZpZ3VyZWQgaW4gWDg2IGFyY2gu
IEFNRCBmdXR1cmUgIEFSTTY0IHByb2Nlc3NvcnMgbWF5DQphbHNvIG5lZWQgYWNwaSB0byBwbGF0
Zm9ybSBzdXBwb3J0LiANCiAgDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1302575

FromBorislav Petkov <bp@alien8.de>
Date2016-01-06 11:50 +0100
Message-ID<qNTPe-8o8-63@gated-at.bofh.it>
In reply to#1302337
On Wed, Jan 06, 2016 at 02:08:18AM +0000, Wang, Annie wrote:
> How about I add select ARM_AMBA and SERIAL_8250 in arch/x86/Kconfig?

Yeah, select sounds good in that case, except in that particular case ...

> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index db3622f..0fe6657 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -537,11 +537,15 @@ config X86_AMD_PLATFORM_DEVICE
>         depends on ACPI
>         select COMMON_CLK
>         select PINCTRL
> +       select SERIAL_8250
> +       select ARM_AMBA

... that's a X86_AMD_PLATFORM_DEVICE which selects ARM thing? i.e.,
ARM_AMBA. Can that even work?

[ Rant on the side: And that ARM_AMBA thing has, of course, no effing
  help text. Dammit, people need to start explaining those cryptic
  abbreviations. Somewhere in the code I found "Advanced Microcontroller
  Bus Architecture". This is clearly suboptimal. ]

So why does the X86 platform device need to select the AMBA crap?

>         ---help---
>           Select to interpret AMD specific ACPI device to platform device
>           such as I2C, UART, GPIO found on AMD Carrizo and later chipsets.
>           I2C and UART depend on COMMON_CLK to set clock. GPIO driver is
> -         implemented under PINCTRL subsystem.
> +         implemented under PINCTRL subsystem. Carrizo's UART is implemented
> +         under SERIAL_8250. Carrizo's UART DMA device is an amba device,
> +         it selects ARM_AMBA option.

As I already said before, please refrain from using platform names like
Carrizo because people have no clue what those are. Only the marketing
people do. Use CPU family + models instead.

> CPU_SUP_AMD is only configured in X86 arch. AMD future  ARM64 processors may
> also need acpi to platform support.

So this code is going to be shared between X86 and ARM64?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1305910

From"Wang, Annie" <Annie.Wang@amd.com>
Date2016-01-11 08:30 +0100
Message-ID<qPF5o-7yx-11@gated-at.bofh.it>
In reply to#1302575

>-----Original Message-----
>From: Borislav Petkov [mailto:bp@alien8.de]
>Sent: Wednesday, January 06, 2016 6:46 PM
>To: Wang, Annie
>Cc: Andy Shevchenko; Vinod Koul; Mika Westerberg; Greg Kroah-Hartman; Rafael
>J. Wysocki; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>serial@vger.kernel.org; dmaengine@vger.kernel.org; Huang, Ray; Wan, Vincent;
>Xue, Ken; Robin Murphy; Graeme Gregory; Li, Tony; Yu, Xiangliang
>Subject: Re: [PATCH 1/6] 8250/Kconfig: add config option
>CONFIG_SERIAL_8250_AMD
>
>On Wed, Jan 06, 2016 at 02:08:18AM +0000, Wang, Annie wrote:
>> How about I add select ARM_AMBA and SERIAL_8250 in arch/x86/Kconfig?
>
>Yeah, select sounds good in that case, except in that particular case ...
>
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
>> db3622f..0fe6657 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -537,11 +537,15 @@ config X86_AMD_PLATFORM_DEVICE
>>         depends on ACPI
>>         select COMMON_CLK
>>         select PINCTRL
>> +       select SERIAL_8250
>> +       select ARM_AMBA
>
>... that's a X86_AMD_PLATFORM_DEVICE which selects ARM thing? i.e.,
>ARM_AMBA. Can that even work?
>
>[ Rant on the side: And that ARM_AMBA thing has, of course, no effing
>  help text. Dammit, people need to start explaining those cryptic
>  abbreviations. Somewhere in the code I found "Advanced Microcontroller
>  Bus Architecture". This is clearly suboptimal. ]
>
>So why does the X86 platform device need to select the AMBA crap?


Russell, 

The AMBA bus is already leveraged  in AMD X86 arch hardware design for UART
controller and UART DMA. And may will be used in other arch as well, however,
it is rather confusing if we select ARM_AMBA in other arch, such as X86.

How about rename  CONFIG_ARM_AMBA to CONFIG_AMBA? So different arch
can select it without causing misunderstanding. 

Thank you very much.
Regards,
Hongcheng(Annie)

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web