All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
To: David Daney <ddaney@caviumnetworks.com>
Cc: Robert Richter <rric@kernel.org>, Tejun Heo <tj@kernel.org>,
	linux-ide@vger.kernel.org, stripathi@apm.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V2] AHCI: Workaround for ThunderX Errata#22536
Date: Tue, 16 Feb 2016 11:50:44 -0800	[thread overview]
Message-ID: <56C37D94.8080403@caviumnetworks.com> (raw)
In-Reply-To: <56C37AD3.4030407@caviumnetworks.com>



On 02/16/2016 11:38 AM, David Daney wrote:
> On 02/16/2016 11:14 AM, Tirumalesh Chalamarla wrote:
>>
>>
>> On 02/16/2016 06:42 AM, Robert Richter wrote:
>>> On 15.02.16 13:30:41, Tejun Heo wrote:
>>>> On Sun, Feb 14, 2016 at 07:36:18PM -0800, Tirumalesh Chalamarla wrote:
>>>>> There is no need for special Driver, AHCI is sufficient for
>>>>> ThunderX, the
>>>>> file only contains this interrupt handler,
>>>>> is it preferable if this interrupt handler in libahci.c with others,
>>>>> instead
>>>>> of separate file?
>>>>
>>>> Yeap, just fold it in ahci.c with surrounding #ifdef guard.
>>>
>>> Yes, please use #ifdef CONFIG_CAVIUM_ERRATUM_22536 ... and add a
>>> kconfig entry for this to arch/arm64/Kconfig.
>>>
>> Are you sure, this is not a workaround that is based on alternative
>> framework rather on pci device and vendor
>>
>> do you think CONFIG_ARCH_THUNDER a good alternative?
>
> No.  CONFIG_ARCH_THUNDER should be removed all together.
>
> Grouping a bunch of unrelated features under a single config variable
> creates a very brittle system.  What are you going to do when a new
> hardware revision is released?  Create CONFIG_ARCH_THUNDER2?  Which one
> of these two would you select if building a kernel?  It is a choice that
> we don't want to force users (kernel builders) to have to waste mental
> energy on.
>
> Instead, let's try to make things work out of the box without having to
> set a bunch of random config variables.
>
> If a generic arm64 kernel won't get too bloated, I would suggest just
> enabling the compilation of the code unconditionally (at least for
> arm64).  The use of the code would still be gated by the PCI version
> probe that is part of the patch.
>

exactly, that is my initial choice with v1, and only depends on vendor 
and device id.

but it seems a config is needed. how about ARCH_ARM64 then?

Thanks,
Tirumalesh.

> David Daney
>

WARNING: multiple messages have this Message-ID (diff)
From: tchalamarla@caviumnetworks.com (Tirumalesh Chalamarla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] AHCI: Workaround for ThunderX Errata#22536
Date: Tue, 16 Feb 2016 11:50:44 -0800	[thread overview]
Message-ID: <56C37D94.8080403@caviumnetworks.com> (raw)
In-Reply-To: <56C37AD3.4030407@caviumnetworks.com>



On 02/16/2016 11:38 AM, David Daney wrote:
> On 02/16/2016 11:14 AM, Tirumalesh Chalamarla wrote:
>>
>>
>> On 02/16/2016 06:42 AM, Robert Richter wrote:
>>> On 15.02.16 13:30:41, Tejun Heo wrote:
>>>> On Sun, Feb 14, 2016 at 07:36:18PM -0800, Tirumalesh Chalamarla wrote:
>>>>> There is no need for special Driver, AHCI is sufficient for
>>>>> ThunderX, the
>>>>> file only contains this interrupt handler,
>>>>> is it preferable if this interrupt handler in libahci.c with others,
>>>>> instead
>>>>> of separate file?
>>>>
>>>> Yeap, just fold it in ahci.c with surrounding #ifdef guard.
>>>
>>> Yes, please use #ifdef CONFIG_CAVIUM_ERRATUM_22536 ... and add a
>>> kconfig entry for this to arch/arm64/Kconfig.
>>>
>> Are you sure, this is not a workaround that is based on alternative
>> framework rather on pci device and vendor
>>
>> do you think CONFIG_ARCH_THUNDER a good alternative?
>
> No.  CONFIG_ARCH_THUNDER should be removed all together.
>
> Grouping a bunch of unrelated features under a single config variable
> creates a very brittle system.  What are you going to do when a new
> hardware revision is released?  Create CONFIG_ARCH_THUNDER2?  Which one
> of these two would you select if building a kernel?  It is a choice that
> we don't want to force users (kernel builders) to have to waste mental
> energy on.
>
> Instead, let's try to make things work out of the box without having to
> set a bunch of random config variables.
>
> If a generic arm64 kernel won't get too bloated, I would suggest just
> enabling the compilation of the code unconditionally (at least for
> arm64).  The use of the code would still be gated by the PCI version
> probe that is part of the patch.
>

exactly, that is my initial choice with v1, and only depends on vendor 
and device id.

but it seems a config is needed. how about ARCH_ARM64 then?

Thanks,
Tirumalesh.

> David Daney
>

WARNING: multiple messages have this Message-ID (diff)
From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
To: David Daney <ddaney@caviumnetworks.com>
Cc: Robert Richter <rric@kernel.org>, Tejun Heo <tj@kernel.org>,
	<linux-ide@vger.kernel.org>, <stripathi@apm.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH V2] AHCI: Workaround for ThunderX Errata#22536
Date: Tue, 16 Feb 2016 11:50:44 -0800	[thread overview]
Message-ID: <56C37D94.8080403@caviumnetworks.com> (raw)
In-Reply-To: <56C37AD3.4030407@caviumnetworks.com>



On 02/16/2016 11:38 AM, David Daney wrote:
> On 02/16/2016 11:14 AM, Tirumalesh Chalamarla wrote:
>>
>>
>> On 02/16/2016 06:42 AM, Robert Richter wrote:
>>> On 15.02.16 13:30:41, Tejun Heo wrote:
>>>> On Sun, Feb 14, 2016 at 07:36:18PM -0800, Tirumalesh Chalamarla wrote:
>>>>> There is no need for special Driver, AHCI is sufficient for
>>>>> ThunderX, the
>>>>> file only contains this interrupt handler,
>>>>> is it preferable if this interrupt handler in libahci.c with others,
>>>>> instead
>>>>> of separate file?
>>>>
>>>> Yeap, just fold it in ahci.c with surrounding #ifdef guard.
>>>
>>> Yes, please use #ifdef CONFIG_CAVIUM_ERRATUM_22536 ... and add a
>>> kconfig entry for this to arch/arm64/Kconfig.
>>>
>> Are you sure, this is not a workaround that is based on alternative
>> framework rather on pci device and vendor
>>
>> do you think CONFIG_ARCH_THUNDER a good alternative?
>
> No.  CONFIG_ARCH_THUNDER should be removed all together.
>
> Grouping a bunch of unrelated features under a single config variable
> creates a very brittle system.  What are you going to do when a new
> hardware revision is released?  Create CONFIG_ARCH_THUNDER2?  Which one
> of these two would you select if building a kernel?  It is a choice that
> we don't want to force users (kernel builders) to have to waste mental
> energy on.
>
> Instead, let's try to make things work out of the box without having to
> set a bunch of random config variables.
>
> If a generic arm64 kernel won't get too bloated, I would suggest just
> enabling the compilation of the code unconditionally (at least for
> arm64).  The use of the code would still be gated by the PCI version
> probe that is part of the patch.
>

exactly, that is my initial choice with v1, and only depends on vendor 
and device id.

but it seems a config is needed. how about ARCH_ARM64 then?

Thanks,
Tirumalesh.

> David Daney
>

  reply	other threads:[~2016-02-16 19:50 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 23:20 [PATCH V2] AHCI: Workaround for ThunderX Errata#22536 tchalamarla
2016-02-12 23:20 ` tchalamarla
2016-02-12 23:20 ` tchalamarla at caviumnetworks.com
2016-02-13 16:54 ` Sergei Shtylyov
2016-02-13 16:54   ` Sergei Shtylyov
2016-02-14 17:01 ` Tejun Heo
2016-02-14 17:01   ` Tejun Heo
2016-02-15  3:36   ` Tirumalesh Chalamarla
2016-02-15  3:36     ` Tirumalesh Chalamarla
2016-02-15  3:36     ` Tirumalesh Chalamarla
2016-02-15 18:30     ` Tejun Heo
2016-02-15 18:30       ` Tejun Heo
2016-02-16 14:42       ` Robert Richter
2016-02-16 14:42         ` Robert Richter
2016-02-16 19:14         ` Tirumalesh Chalamarla
2016-02-16 19:14           ` Tirumalesh Chalamarla
2016-02-16 19:14           ` Tirumalesh Chalamarla
2016-02-16 19:38           ` David Daney
2016-02-16 19:38             ` David Daney
2016-02-16 19:38             ` David Daney
2016-02-16 19:50             ` Tirumalesh Chalamarla [this message]
2016-02-16 19:50               ` Tirumalesh Chalamarla
2016-02-16 19:50               ` Tirumalesh Chalamarla
2016-02-16 21:14               ` Robert Richter
2016-02-16 21:14                 ` Robert Richter
2016-02-16 23:13                 ` Tirumalesh Chalamarla
2016-02-16 23:13                   ` Tirumalesh Chalamarla
2016-02-16 23:13                   ` Tirumalesh Chalamarla
2016-02-17 11:29                   ` Will Deacon
2016-02-17 11:29                     ` Will Deacon
2016-02-17 18:57                     ` David Daney
2016-02-17 18:57                       ` David Daney
2016-02-17 18:57                       ` David Daney
2016-02-17 20:00                       ` Tejun Heo
2016-02-17 20:00                         ` Tejun Heo
2016-02-17 21:46                         ` Tirumalesh Chalamarla
2016-02-17 21:46                           ` Tirumalesh Chalamarla
2016-02-17 21:46                           ` Tirumalesh Chalamarla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C37D94.8080403@caviumnetworks.com \
    --to=tchalamarla@caviumnetworks.com \
    --cc=ddaney@caviumnetworks.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rric@kernel.org \
    --cc=stripathi@apm.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.