All of lore.kernel.org
 help / color / mirror / Atom feed
From: grinberg@compulab.co.il (Igor Grinberg)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ARM: dts: imx6q: extend support for the cm-fx6
Date: Sun, 5 Jun 2016 19:43:51 +0300	[thread overview]
Message-ID: <575456C7.9090709@compulab.co.il> (raw)
In-Reply-To: <5746DFE2.9040903@compulab.co.il>

Hi Christopher,

On 05/26/2016 02:37 PM, Igor Grinberg wrote:
> On 05/26/2016 12:50 PM, Lucas Stach wrote:
>> Hi Igor,
>>
>> Am Donnerstag, den 26.05.2016, 11:50 +0300 schrieb Igor Grinberg:
>>> Hi Lucas,
>>>
>>> Thanks for reviewing the patch(es).
>>>
>>> On 05/23/2016 12:03 PM, Lucas Stach wrote:
>>>> Am Montag, den 23.05.2016, 00:47 +0200 schrieb
>>>> christopher.spinrath at rwth-aachen.de:
>>>>> From: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
>>>
>>> [...]
>>>
>>>>> +&ecspi1 {
>>>>> +	fsl,spi-num-chipselects = <2>;
>>>>> +	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>;
>>>>> +	pinctrl-names = "default";
>>>>> +	pinctrl-0 = <&pinctrl_ecspi1>;
>>>>> +	status = "okay";
>>>>> +
>>>>> +	flash: m25p80 at 0 {
>>>>> +		#address-cells = <1>;
>>>>> +		#size-cells = <1>;
>>>>> +		compatible = "st,m25p", "jedec,spi-nor";
>>>>> +		spi-max-frequency = <20000000>;
>>>>> +		reg = <0>;
>>>>> +
>>>>> +		partition at 0 {
>>>>> +			label = "uboot";
>>>>> +			reg = <0x0 0xc0000>;
>>>>> +		};
>>>>> +
>>>>> +		partition at c0000 {
>>>>> +			label = "uboot environment";
>>>>> +			reg = <0xc0000 0x40000>;
>>>>> +		};
>>>>> +
>>>>> +		partition at 100000 {
>>>>> +			label = "reserved";
>>>>> +			reg = <0x100000 0x100000>;
>>>>> +		};
>>>>
>>>> Partition layouts don't belong in the upstream DT, as it a device
>>>> configuration thing. Please kep them in the bootloader/firmware and make
>>>> this one pass the partition layout to the kernel.
>>>
>>> I don't completely agree with this.
>>> We have lots of partition layouts in the upstream DT.
>>
>> No, we don't. At least not for the i.MX6. There are some for the earlier
>> i.MX boards, but IMO it's wrong to put device configuration into the
>> upstream DT. Let's not start doing this again.
> 
> Why not?
> For i.MX6 there are 2 boards that have the partitioning scheme.
> I'm not considering this a device configuration, but rather
> a default partitioning layout/scheme.
> Current case is for the firmware storage device that is not likely
> to change.
> Moreover, a DT is not really a part of the kernel, but lays along the kernel
> sources for convenience and simplicity (at least IIRC as it was decided
> about 5 years ago). It is more a part of the firmware for a device, than
> an upstream kernel source code.
> I think it is only a meter of time when Linus will decide that he does not
> want it inside the kernel anymore...
> 
>>
>>> Moreover, this is the default layout and changing it, will
>>> result in incompatibilities and also might result in device "bricking".
>>> Those can be changed from the boot loader in case you need those
>>> the other way around.
>>> Another question of mine is, why should you?
>>>
>> Partition layout is device configuration, which is governed by the
>> device firmware.
> 
> Yet again, DT is a part of device firmware.
> Moreover, the firmware (in that case U-Boot), can be configured
> using the very same DT code, so not having this in, might force
> various w/a and hacks.
> 
>> By not having the partition layout in the upstream DT
>> people are forced to set it from the firmware, which is exactly the
>> right thing to do, weather or not you plan to change it at any time.
> 
> I might be ignorant, sorry for that.
> Why? Why is it right and why would you want to force people to do that?
> 
> 

No answer?
I think it is worth keeping this as a default firmware layout.


-- 
Regards,
Igor.

WARNING: multiple messages have this Message-ID (diff)
From: Igor Grinberg <grinberg-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
To: christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org
Cc: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 1/2] ARM: dts: imx6q: extend support for the cm-fx6
Date: Sun, 5 Jun 2016 19:43:51 +0300	[thread overview]
Message-ID: <575456C7.9090709@compulab.co.il> (raw)
In-Reply-To: <5746DFE2.9040903-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>

Hi Christopher,

On 05/26/2016 02:37 PM, Igor Grinberg wrote:
> On 05/26/2016 12:50 PM, Lucas Stach wrote:
>> Hi Igor,
>>
>> Am Donnerstag, den 26.05.2016, 11:50 +0300 schrieb Igor Grinberg:
>>> Hi Lucas,
>>>
>>> Thanks for reviewing the patch(es).
>>>
>>> On 05/23/2016 12:03 PM, Lucas Stach wrote:
>>>> Am Montag, den 23.05.2016, 00:47 +0200 schrieb
>>>> christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org:
>>>>> From: Christopher Spinrath <christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
>>>
>>> [...]
>>>
>>>>> +&ecspi1 {
>>>>> +	fsl,spi-num-chipselects = <2>;
>>>>> +	cs-gpios = <&gpio2 30 0>, <&gpio3 19 0>;
>>>>> +	pinctrl-names = "default";
>>>>> +	pinctrl-0 = <&pinctrl_ecspi1>;
>>>>> +	status = "okay";
>>>>> +
>>>>> +	flash: m25p80@0 {
>>>>> +		#address-cells = <1>;
>>>>> +		#size-cells = <1>;
>>>>> +		compatible = "st,m25p", "jedec,spi-nor";
>>>>> +		spi-max-frequency = <20000000>;
>>>>> +		reg = <0>;
>>>>> +
>>>>> +		partition@0 {
>>>>> +			label = "uboot";
>>>>> +			reg = <0x0 0xc0000>;
>>>>> +		};
>>>>> +
>>>>> +		partition@c0000 {
>>>>> +			label = "uboot environment";
>>>>> +			reg = <0xc0000 0x40000>;
>>>>> +		};
>>>>> +
>>>>> +		partition@100000 {
>>>>> +			label = "reserved";
>>>>> +			reg = <0x100000 0x100000>;
>>>>> +		};
>>>>
>>>> Partition layouts don't belong in the upstream DT, as it a device
>>>> configuration thing. Please kep them in the bootloader/firmware and make
>>>> this one pass the partition layout to the kernel.
>>>
>>> I don't completely agree with this.
>>> We have lots of partition layouts in the upstream DT.
>>
>> No, we don't. At least not for the i.MX6. There are some for the earlier
>> i.MX boards, but IMO it's wrong to put device configuration into the
>> upstream DT. Let's not start doing this again.
> 
> Why not?
> For i.MX6 there are 2 boards that have the partitioning scheme.
> I'm not considering this a device configuration, but rather
> a default partitioning layout/scheme.
> Current case is for the firmware storage device that is not likely
> to change.
> Moreover, a DT is not really a part of the kernel, but lays along the kernel
> sources for convenience and simplicity (at least IIRC as it was decided
> about 5 years ago). It is more a part of the firmware for a device, than
> an upstream kernel source code.
> I think it is only a meter of time when Linus will decide that he does not
> want it inside the kernel anymore...
> 
>>
>>> Moreover, this is the default layout and changing it, will
>>> result in incompatibilities and also might result in device "bricking".
>>> Those can be changed from the boot loader in case you need those
>>> the other way around.
>>> Another question of mine is, why should you?
>>>
>> Partition layout is device configuration, which is governed by the
>> device firmware.
> 
> Yet again, DT is a part of device firmware.
> Moreover, the firmware (in that case U-Boot), can be configured
> using the very same DT code, so not having this in, might force
> various w/a and hacks.
> 
>> By not having the partition layout in the upstream DT
>> people are forced to set it from the firmware, which is exactly the
>> right thing to do, weather or not you plan to change it at any time.
> 
> I might be ignorant, sorry for that.
> Why? Why is it right and why would you want to force people to do that?
> 
> 

No answer?
I think it is worth keeping this as a default firmware layout.


-- 
Regards,
Igor.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-06-05 16:43 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-22 22:47 [PATCH 1/2] ARM: dts: imx6q: extend support for the cm-fx6 christopher.spinrath at rwth-aachen.de
2016-05-22 22:47 ` christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg
2016-05-23  9:03 ` Lucas Stach
2016-05-23  9:03   ` Lucas Stach
2016-05-26  8:50   ` Igor Grinberg
2016-05-26  8:50     ` Igor Grinberg
2016-05-26  9:50     ` Lucas Stach
2016-05-26  9:50       ` Lucas Stach
2016-05-26 11:37       ` Igor Grinberg
2016-05-26 11:37         ` Igor Grinberg
2016-06-05 16:43         ` Igor Grinberg [this message]
2016-06-05 16:43           ` Igor Grinberg
     [not found]         ` <d083d0308abd48efbd4b6042e1f9c470@rwthex-s2-a.rwth-ad.de>
2016-06-05 17:32           ` Christopher Spinrath
2016-06-05 17:32             ` Christopher Spinrath
2016-06-06  6:49             ` Igor Grinberg
2016-06-06  6:49               ` Igor Grinberg
2016-06-06 10:37               ` Christopher Spinrath
2016-06-06 10:37                 ` Christopher Spinrath
2016-06-07  8:38                 ` Igor Grinberg
2016-06-07  8:38                   ` Igor Grinberg
     [not found]                 ` <5d1bc50e59f749db8f825f5cb7e2438d@rwthex-w2-a.rwth-ad.de>
2016-06-07  9:45                   ` Christopher Spinrath
2016-06-07  9:45                     ` Christopher Spinrath
2016-06-08  8:11                     ` Igor Grinberg
2016-06-08  8:11                       ` Igor Grinberg
     [not found] ` <1c18d1b1f80b47cab2d06b1167407ae8@rwthex-s2-a.rwth-ad.de>
2016-05-23 22:37   ` Christopher Spinrath
2016-05-23 22:37     ` Christopher Spinrath

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=575456C7.9090709@compulab.co.il \
    --to=grinberg@compulab.co.il \
    --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.