From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Horman <horms@verge.net.au>
Subject: Re: [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
Date: Mon, 28 May 2018 11:36:03 +0200
Message-ID: <20180528093602.eb36j34bbfzmpmw2@verge.net.au>
References: <20180522120257.13232-1-marek.vasut+renesas@gmail.com>
 <20180522120257.13232-6-marek.vasut+renesas@gmail.com>
 <CAMuHMdXRnoyhNz=3v_=ZCV=FWX4TwKqx7MpH0g3c7GwtXKP1tg@mail.gmail.com>
 <fc1cfd7e-db27-9277-dbed-cc2274c614a6@gmail.com>
 <CAMuHMdXnNABJSJZ6MLF+aQu8QKpFoMCAvno9Yc-9LOe810NtbA@mail.gmail.com>
 <7e1563c3-3e13-235b-3071-c3936502530f@gmail.com>
 <20180528084135.q5xng5eaypprsypr@verge.net.au>
 <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org>
Content-Disposition: inline
In-Reply-To: <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>,
 <mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
List-Post: <mailto:linux-arm-kernel@lists.infradead.org>
List-Help: <mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
 <mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>
Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org>
Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, Geert Uytterhoeven <geert+renesas@glider.be>, Wolfram Sang <wsa@the-dreams.de>, Richard Weinberger <richard@nod.at>, Boris Brezillon <boris.brezillon@bootlin.com>, Linux-Renesas <linux-renesas-soc@vger.kernel.org>, Marek Vasut <marek.vasut@gmail.com>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Marek Vasut <marek.vasut+renesas@gmail.com>
List-Id: devicetree@vger.kernel.org

On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote:
> >> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote:
> >> > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote:
> >> >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >>>> Drop the MTD partitioning from DT, since it does not describe HW
> >> >>>> and to give way to a more flexible kernel command line partition
> >> >>>> passing.
> >> >>>>
> >> >>>> To retain the original partitioning, assure you have enabled
> >> >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> >> >>>> following to your kernel command line:
> >> >>>>
> >> >>>>   mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash)
> >> >>>
> >> >>> I think the "@0" can be dropped, as it's optional?
> >> >>> 4m?
> >> >>
> >> >> My take on this is that the loader is actually at offset 0x0 of the MTD
> >> >> device and we explicitly state that in the mtdparts to anchor the first
> >> >> partition within the MTD device and all the other partitions are at
> >> >> offset +(sum of the sizes of all partitions listed before the current
> >> >> one) relative to that first partition.
> >> >
> >> > Where is this explicitly states for the first partition?
> >> >
> >> >> Removing the @0 feels fragile at best and it seems to depend on the
> >> >> current behavior of the code.
> >> >
> >> > Better, it also depends on the documented behavior:
> >> >
> >> > Documentation/admin-guide/kernel-parameters.txt refers to
> >> > drivers/mtd/cmdlinepart.c, which states:
> >> >
> >> >  * <offset>  := standard linux memsize
> >> >  *              if omitted the part will immediately follow the previous part
> >> >  *              or 0 if the first part
> >> >
> >> > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig
> >> > help text, or in a defconfig bundled with the kernel, use @0 for the first
> >> > partition.
> >>
> >> I think this is exceptionally fragile and dangerous to depend on this,
> >> but so be it.
> >
> > Could you respin with this change?
> >
> > I would also like to ask for another change, in light of recent
> > feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT
> > Updates for v4.18").
> >
> > Please consolidate the dts patches into a single patch?
> 
> I think it's better to keep them split, as each commit description mentions
> what needs to be passed on the kernel command line for the corresponding
> board.
> 
> Combining it in a single patch makes it much harder to extract this information.
> Unless you're fine with a list:
> 
>    koelsch: ...
>    wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash)

Lets try a list.

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-renesas-soc-owner@vger.kernel.org>
Received: from kirsty.vergenet.net ([202.4.237.240]:56691 "EHLO
        kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S932104AbeE1JgJ (ORCPT
        <rfc822;linux-renesas-soc@vger.kernel.org>);
        Mon, 28 May 2018 05:36:09 -0400
Date: Mon, 28 May 2018 11:36:03 +0200
From: Simon Horman <horms@verge.net.au>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Marek Vasut <marek.vasut@gmail.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
        <devicetree@vger.kernel.org>,
        Marek Vasut <marek.vasut+renesas@gmail.com>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Wolfram Sang <wsa@the-dreams.de>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        Richard Weinberger <richard@nod.at>,
        Boris Brezillon <boris.brezillon@bootlin.com>
Subject: Re: [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
Message-ID: <20180528093602.eb36j34bbfzmpmw2@verge.net.au>
References: <20180522120257.13232-1-marek.vasut+renesas@gmail.com>
 <20180522120257.13232-6-marek.vasut+renesas@gmail.com>
 <CAMuHMdXRnoyhNz=3v_=ZCV=FWX4TwKqx7MpH0g3c7GwtXKP1tg@mail.gmail.com>
 <fc1cfd7e-db27-9277-dbed-cc2274c614a6@gmail.com>
 <CAMuHMdXnNABJSJZ6MLF+aQu8QKpFoMCAvno9Yc-9LOe810NtbA@mail.gmail.com>
 <7e1563c3-3e13-235b-3071-c3936502530f@gmail.com>
 <20180528084135.q5xng5eaypprsypr@verge.net.au>
 <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
Sender: linux-renesas-soc-owner@vger.kernel.org
List-ID: <linux-renesas-soc.vger.kernel.org>

On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote:
> >> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote:
> >> > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote:
> >> >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >>>> Drop the MTD partitioning from DT, since it does not describe HW
> >> >>>> and to give way to a more flexible kernel command line partition
> >> >>>> passing.
> >> >>>>
> >> >>>> To retain the original partitioning, assure you have enabled
> >> >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> >> >>>> following to your kernel command line:
> >> >>>>
> >> >>>>   mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash)
> >> >>>
> >> >>> I think the "@0" can be dropped, as it's optional?
> >> >>> 4m?
> >> >>
> >> >> My take on this is that the loader is actually at offset 0x0 of the MTD
> >> >> device and we explicitly state that in the mtdparts to anchor the first
> >> >> partition within the MTD device and all the other partitions are at
> >> >> offset +(sum of the sizes of all partitions listed before the current
> >> >> one) relative to that first partition.
> >> >
> >> > Where is this explicitly states for the first partition?
> >> >
> >> >> Removing the @0 feels fragile at best and it seems to depend on the
> >> >> current behavior of the code.
> >> >
> >> > Better, it also depends on the documented behavior:
> >> >
> >> > Documentation/admin-guide/kernel-parameters.txt refers to
> >> > drivers/mtd/cmdlinepart.c, which states:
> >> >
> >> >  * <offset>  := standard linux memsize
> >> >  *              if omitted the part will immediately follow the previous part
> >> >  *              or 0 if the first part
> >> >
> >> > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig
> >> > help text, or in a defconfig bundled with the kernel, use @0 for the first
> >> > partition.
> >>
> >> I think this is exceptionally fragile and dangerous to depend on this,
> >> but so be it.
> >
> > Could you respin with this change?
> >
> > I would also like to ask for another change, in light of recent
> > feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT
> > Updates for v4.18").
> >
> > Please consolidate the dts patches into a single patch?
> 
> I think it's better to keep them split, as each commit description mentions
> what needs to be passed on the kernel command line for the corresponding
> board.
> 
> Combining it in a single patch makes it much harder to extract this information.
> Unless you're fine with a list:
> 
>    koelsch: ...
>    wheat: mtdparts=spi0.0:256k@0(loader),4096k(user),-(flash)

Lets try a list.

From mboxrd@z Thu Jan  1 00:00:00 1970
From: horms@verge.net.au (Simon Horman)
Date: Mon, 28 May 2018 11:36:03 +0200
Subject: [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
In-Reply-To: <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
References: <20180522120257.13232-1-marek.vasut+renesas@gmail.com>
 <20180522120257.13232-6-marek.vasut+renesas@gmail.com>
 <CAMuHMdXRnoyhNz=3v_=ZCV=FWX4TwKqx7MpH0g3c7GwtXKP1tg@mail.gmail.com>
 <fc1cfd7e-db27-9277-dbed-cc2274c614a6@gmail.com>
 <CAMuHMdXnNABJSJZ6MLF+aQu8QKpFoMCAvno9Yc-9LOe810NtbA@mail.gmail.com>
 <7e1563c3-3e13-235b-3071-c3936502530f@gmail.com>
 <20180528084135.q5xng5eaypprsypr@verge.net.au>
 <CAMuHMdXYyN=XY6nb9RVEE+t0NeoSPhTh6fUk-9YWdFzAE1FfPA@mail.gmail.com>
Message-ID: <20180528093602.eb36j34bbfzmpmw2@verge.net.au>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Mon, May 28, 2018 at 10:54:57AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, May 28, 2018 at 10:41 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, May 24, 2018 at 04:52:59PM +0200, Marek Vasut wrote:
> >> On 05/23/2018 08:25 AM, Geert Uytterhoeven wrote:
> >> > On Wed, May 23, 2018 at 12:01 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >> On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote:
> >> >>> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >>>> Drop the MTD partitioning from DT, since it does not describe HW
> >> >>>> and to give way to a more flexible kernel command line partition
> >> >>>> passing.
> >> >>>>
> >> >>>> To retain the original partitioning, assure you have enabled
> >> >>>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> >> >>>> following to your kernel command line:
> >> >>>>
> >> >>>>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)
> >> >>>
> >> >>> I think the "@0" can be dropped, as it's optional?
> >> >>> 4m?
> >> >>
> >> >> My take on this is that the loader is actually at offset 0x0 of the MTD
> >> >> device and we explicitly state that in the mtdparts to anchor the first
> >> >> partition within the MTD device and all the other partitions are at
> >> >> offset +(sum of the sizes of all partitions listed before the current
> >> >> one) relative to that first partition.
> >> >
> >> > Where is this explicitly states for the first partition?
> >> >
> >> >> Removing the @0 feels fragile at best and it seems to depend on the
> >> >> current behavior of the code.
> >> >
> >> > Better, it also depends on the documented behavior:
> >> >
> >> > Documentation/admin-guide/kernel-parameters.txt refers to
> >> > drivers/mtd/cmdlinepart.c, which states:
> >> >
> >> >  * <offset>  := standard linux memsize
> >> >  *              if omitted the part will immediately follow the previous part
> >> >  *              or 0 if the first part
> >> >
> >> > None of the examples listed there or under the MTD_CMDLINE_PARTS Kconfig
> >> > help text, or in a defconfig bundled with the kernel, use @0 for the first
> >> > partition.
> >>
> >> I think this is exceptionally fragile and dangerous to depend on this,
> >> but so be it.
> >
> > Could you respin with this change?
> >
> > I would also like to ask for another change, in light of recent
> > feedback from Olof Johansson ("Re: [GIT PULL] Renesas ARM64 Based SoC DT
> > Updates for v4.18").
> >
> > Please consolidate the dts patches into a single patch?
> 
> I think it's better to keep them split, as each commit description mentions
> what needs to be passed on the kernel command line for the corresponding
> board.
> 
> Combining it in a single patch makes it much harder to extract this information.
> Unless you're fine with a list:
> 
>    koelsch: ...
>    wheat: mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

Lets try a list.