All of lore.kernel.org
 help / color / mirror / Atom feed
From: bintian.wang@huawei.com (Bintian)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v9 4/6] Documentation: DT: PL011: hi6220: add compatible string for Hisilicon designed UART
Date: Tue, 2 Jun 2015 19:46:07 +0800	[thread overview]
Message-ID: <556D977F.3000904@huawei.com> (raw)
In-Reply-To: <20150602112424.GC2067@n2100.arm.linux.org.uk>

Hello Russell,

On 2015/6/2 19:24, Russell King - ARM Linux wrote:
> On Tue, Jun 02, 2015 at 06:55:20PM +0800, Bintian wrote:
>> On 2015/6/2 16:59, Linus Walleij wrote:
>>> On Sat, May 30, 2015 at 3:50 AM, Bintian Wang <bintian.wang@huawei.com> wrote:
>>>
>>>> Hisilicon does some performance enhancements based on PL011(e.g. larger
>>>> FIFO length), so add one compatible string "hisilicon,hi6220-uart" for
>>>
>>> That compatible string in the commit message is not even
>>> the same as in the patch.
>> The UART0 is PL011 compatible, the UART1/2 have some performance
>> enhancements features, so based on Mark's suggestion and I add this
>> compatible string just for future use.
>
> Please don't submit it with this series.
>
> This patch should not be part of this series, it should be part of the
> series which modifies the PL011 driver, so it can be reviewed along with
> those changes.
I agree with you and it's OK to me to remove this patch now.

Could you help to ack the reset patches or I should send the version 10
without this patch?

>
> Until then, I'm going to NAK this patch.
>
> The thing that worries me though is that the subject line says this
> is a "Hisilicon *designed* UART".  If Hisilicon _designed_ this UART,
> presumably they have changed the *vendor* field of the UART ID _not_
> to indicate that ARM Ltd designed it?
 >
> If they've merely modified the parameters, and given the ARM Ltd PL011
> a larger fifo, then there isn't really much of a problem - we've been
> here before, except the vendor has had a real vendor ID for the field
> (in the case of ST), plus we've had different FIFO lengths for ARM
> hardware too (32 bytes instead of 16 for revision 3 and above.)
I think there is problem with my subject description, it's ARM designed
indeed and Hisilicon just did some performance enhancements but not for
UART0 in hi6220.

> Lastly, if you're not having to modify the PL011 driver in any way,
> you don't need to have a compatible.  In any case, you _shouldn't_ for
> AMBA devices.  AMBA does not match drivers based on OF compatible
> strings, so using OF compatible strings with the AMBA bus is just wrong.
> The AMBA compatible strings are there so that the generic OF code knows
> how to create the devices.
Right.

Thank you Russell.

BR,

Bintian

WARNING: multiple messages have this Message-ID (diff)
From: Bintian <bintian.wang@huawei.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will.deacon@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Paweł Moll" <pawel.moll@arm.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Kevin Hilman" <khilman@linaro.org>,
	"Mike Turquette" <mturquette@linaro.org>,
	"Rob Herring" <rob.herring@linaro.org>,
	"Zhangfei Gao" <zhangfei.gao@linaro.org>,
	"Haojian Zhuang" <haojian.zhuang@linaro.org>,
	"Xu Wei" <xuwei5@hisilicon.com>,
	"Jaehoon Chung" <jh80.chung@samsung.com>,
	"Olof Johansson" <olof@lixom.net>,
	yanhaifeng@gmail.com
Subject: Re: [PATCH v9 4/6] Documentation: DT: PL011: hi6220: add compatible string for Hisilicon designed UART
Date: Tue, 2 Jun 2015 19:46:07 +0800	[thread overview]
Message-ID: <556D977F.3000904@huawei.com> (raw)
In-Reply-To: <20150602112424.GC2067@n2100.arm.linux.org.uk>

Hello Russell,

On 2015/6/2 19:24, Russell King - ARM Linux wrote:
> On Tue, Jun 02, 2015 at 06:55:20PM +0800, Bintian wrote:
>> On 2015/6/2 16:59, Linus Walleij wrote:
>>> On Sat, May 30, 2015 at 3:50 AM, Bintian Wang <bintian.wang@huawei.com> wrote:
>>>
>>>> Hisilicon does some performance enhancements based on PL011(e.g. larger
>>>> FIFO length), so add one compatible string "hisilicon,hi6220-uart" for
>>>
>>> That compatible string in the commit message is not even
>>> the same as in the patch.
>> The UART0 is PL011 compatible, the UART1/2 have some performance
>> enhancements features, so based on Mark's suggestion and I add this
>> compatible string just for future use.
>
> Please don't submit it with this series.
>
> This patch should not be part of this series, it should be part of the
> series which modifies the PL011 driver, so it can be reviewed along with
> those changes.
I agree with you and it's OK to me to remove this patch now.

Could you help to ack the reset patches or I should send the version 10
without this patch?

>
> Until then, I'm going to NAK this patch.
>
> The thing that worries me though is that the subject line says this
> is a "Hisilicon *designed* UART".  If Hisilicon _designed_ this UART,
> presumably they have changed the *vendor* field of the UART ID _not_
> to indicate that ARM Ltd designed it?
 >
> If they've merely modified the parameters, and given the ARM Ltd PL011
> a larger fifo, then there isn't really much of a problem - we've been
> here before, except the vendor has had a real vendor ID for the field
> (in the case of ST), plus we've had different FIFO lengths for ARM
> hardware too (32 bytes instead of 16 for revision 3 and above.)
I think there is problem with my subject description, it's ARM designed
indeed and Hisilicon just did some performance enhancements but not for
UART0 in hi6220.

> Lastly, if you're not having to modify the PL011 driver in any way,
> you don't need to have a compatible.  In any case, you _shouldn't_ for
> AMBA devices.  AMBA does not match drivers based on OF compatible
> strings, so using OF compatible strings with the AMBA bus is just wrong.
> The AMBA compatible strings are there so that the generic OF code knows
> how to create the devices.
Right.

Thank you Russell.

BR,

Bintian

WARNING: multiple messages have this Message-ID (diff)
From: Bintian <bintian.wang@huawei.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: "Linus Walleij" <linus.walleij@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will.deacon@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Paweł Moll" <pawel.moll@arm.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Kevin Hilman" <khilman@linaro.org>,
	"Mike Turquette" <mturquette@linaro.org>,
	"Rob Herring" <rob.herring@linaro.org>,
	"Zhangfei Gao" <zhangfei.gao@linaro.org>,
	"Haojian Zhuang" <haojian.zhuang@linaro.org>,
	"Xu Wei" <xuwei5@hisilicon.com>,
	"Jaehoon Chung" <jh80.chung@samsung.com>,
	"Olof Johansson" <olof@lixom.net>,
	yanhaifeng@gmail.com, "Stephen Boyd" <sboyd@codeaurora.org>,
	xuejiancheng@huawei.com, sledge.yanwei@huawei.com,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Guodong Xu" <guodong.xu@linaro.org>,
	jorge.ramirez-ortiz@linaro.org,
	"Tyler Baker" <tyler.baker@linaro.org>,
	"Kevin Hilman" <khilman@kernel.org>,
	"Paul Bolle" <pebolle@tiscali.nl>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Marc Zyngier" <marc.zyngier@arm.com>,
	xuyiping@hisilicon.com, wangbinghui@hisilicon.com,
	zhenwei.wang@hisilicon.com, victor.lixin@hisilicon.com,
	puck.chen@hisilicon.com, dan.zhao@hisilicon.com,
	"Xinwei Hu" <huxinwei@huawei.com>,
	z.liuxinliang@huawei.com, "Yunlei He" <heyunlei@huawei.com>,
	"XinWei Kong" <kong.kongxinwei@hisilicon.com>,
	wangbintian@gmail.com, w.f@huawei.com, liguozhu@hisilicon.com
Subject: Re: [PATCH v9 4/6] Documentation: DT: PL011: hi6220: add compatible string for Hisilicon designed UART
Date: Tue, 2 Jun 2015 19:46:07 +0800	[thread overview]
Message-ID: <556D977F.3000904@huawei.com> (raw)
In-Reply-To: <20150602112424.GC2067@n2100.arm.linux.org.uk>

Hello Russell,

On 2015/6/2 19:24, Russell King - ARM Linux wrote:
> On Tue, Jun 02, 2015 at 06:55:20PM +0800, Bintian wrote:
>> On 2015/6/2 16:59, Linus Walleij wrote:
>>> On Sat, May 30, 2015 at 3:50 AM, Bintian Wang <bintian.wang@huawei.com> wrote:
>>>
>>>> Hisilicon does some performance enhancements based on PL011(e.g. larger
>>>> FIFO length), so add one compatible string "hisilicon,hi6220-uart" for
>>>
>>> That compatible string in the commit message is not even
>>> the same as in the patch.
>> The UART0 is PL011 compatible, the UART1/2 have some performance
>> enhancements features, so based on Mark's suggestion and I add this
>> compatible string just for future use.
>
> Please don't submit it with this series.
>
> This patch should not be part of this series, it should be part of the
> series which modifies the PL011 driver, so it can be reviewed along with
> those changes.
I agree with you and it's OK to me to remove this patch now.

Could you help to ack the reset patches or I should send the version 10
without this patch?

>
> Until then, I'm going to NAK this patch.
>
> The thing that worries me though is that the subject line says this
> is a "Hisilicon *designed* UART".  If Hisilicon _designed_ this UART,
> presumably they have changed the *vendor* field of the UART ID _not_
> to indicate that ARM Ltd designed it?
 >
> If they've merely modified the parameters, and given the ARM Ltd PL011
> a larger fifo, then there isn't really much of a problem - we've been
> here before, except the vendor has had a real vendor ID for the field
> (in the case of ST), plus we've had different FIFO lengths for ARM
> hardware too (32 bytes instead of 16 for revision 3 and above.)
I think there is problem with my subject description, it's ARM designed
indeed and Hisilicon just did some performance enhancements but not for
UART0 in hi6220.

> Lastly, if you're not having to modify the PL011 driver in any way,
> you don't need to have a compatible.  In any case, you _shouldn't_ for
> AMBA devices.  AMBA does not match drivers based on OF compatible
> strings, so using OF compatible strings with the AMBA bus is just wrong.
> The AMBA compatible strings are there so that the generic OF code knows
> how to create the devices.
Right.

Thank you Russell.

BR,

Bintian


  reply	other threads:[~2015-06-02 11:46 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-30  1:50 [PATCH v9 0/6] arm64,hi6220: Enable Hisilicon Hi6220 SoC Bintian Wang
2015-05-30  1:50 ` Bintian Wang
2015-05-30  1:50 ` Bintian Wang
2015-05-30  1:50 ` [PATCH v9 1/6] arm64: Enable Hisilicon ARMv8 SoC family in Kconfig and defconfig Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-06-02  9:03   ` Will Deacon
2015-06-02  9:03     ` Will Deacon
2015-05-30  1:50 ` [PATCH v9 2/6] arm64: hi6220: Document devicetree bindings for Hisilicon hi6220 SoC Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-06-04  4:36   ` Rob Herring
2015-06-04  4:36     ` Rob Herring
2015-06-04  4:36     ` Rob Herring
2015-06-04  7:23     ` Bintian
2015-06-04  7:23       ` Bintian
2015-06-04  7:23       ` Bintian
2015-05-30  1:50 ` [PATCH v9 3/6] clk: hi6220: Document devicetree bindings for hi6220 clock Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-05-30  1:50 ` [PATCH v9 4/6] Documentation: DT: PL011: hi6220: add compatible string for Hisilicon designed UART Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-05-30  1:50   ` Bintian Wang
2015-06-02  8:59   ` Linus Walleij
2015-06-02  8:59     ` Linus Walleij
2015-06-02  8:59     ` Linus Walleij
2015-06-02  9:13     ` Marc Zyngier
2015-06-02  9:13       ` Marc Zyngier
2015-06-02  9:13       ` Marc Zyngier
2015-06-02  9:43       ` Russell King - ARM Linux
2015-06-02  9:43         ` Russell King - ARM Linux
2015-06-02  9:43         ` Russell King - ARM Linux
2015-06-08 13:32       ` Linus Walleij
2015-06-08 13:32         ` Linus Walleij
2015-06-02 10:55     ` Bintian
2015-06-02 10:55       ` Bintian
2015-06-02 10:55       ` Bintian
2015-06-02 11:24       ` Russell King - ARM Linux
2015-06-02 11:24         ` Russell King - ARM Linux
2015-06-02 11:24         ` Russell King - ARM Linux
2015-06-02 11:46         ` Bintian [this message]
2015-06-02 11:46           ` Bintian
2015-06-02 11:46           ` Bintian
2015-05-30  1:51 ` [PATCH v9 5/6] arm64: dts: Add dts files for Hisilicon Hi6220 SoC Bintian Wang
2015-05-30  1:51   ` Bintian Wang
2015-05-30  1:51   ` Bintian Wang
2015-06-03  3:10   ` Bintian
2015-06-03  3:10     ` Bintian
2015-06-03  3:10     ` Bintian
2015-06-09  0:55   ` Shawn Guo
2015-06-09  0:55     ` Shawn Guo
2015-06-09  0:55     ` Shawn Guo
2015-06-09  1:39     ` Bintian
2015-06-09  1:39       ` Bintian
2015-06-09  1:39       ` Bintian
2015-06-09  2:30       ` Bintian
2015-06-09  2:30         ` Bintian
2015-06-09  2:30         ` Bintian
2015-05-30  1:51 ` [PATCH v9 6/6] dt-bindings: Add header file of hi6220 clock driver Bintian Wang
2015-05-30  1:51   ` Bintian Wang
2015-05-30  1:51   ` Bintian Wang
2015-06-02  0:14 ` [PATCH v9 0/6] arm64,hi6220: Enable Hisilicon Hi6220 SoC Kevin Hilman
2015-06-02  0:14   ` Kevin Hilman
2015-06-02  0:14   ` Kevin Hilman
2015-06-02  3:57 ` Bintian
2015-06-02  3:57   ` Bintian
2015-06-02  3:57   ` Bintian
2015-06-02  9:05   ` Will Deacon
2015-06-02  9:05     ` Will Deacon
2015-06-02 10:49     ` Bintian
2015-06-02 10:49       ` Bintian

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=556D977F.3000904@huawei.com \
    --to=bintian.wang@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.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.