by H. Nikolaus Schaller

[permalink] [raw]

Subject: Re: [PATCH 10/32] ARM: dts: omap3-gta04: update gpmc NAND setup

> Am 25.07.2018 um 10:28 schrieb Ladislav Michl <[email protected]>:
>
> On Wed, Jul 25, 2018 at 10:16:28AM +0200, H. Nikolaus Schaller wrote:
>> Hi,
>>
>>> Am 25.07.2018 um 10:10 schrieb Ladislav Michl <[email protected]>:
>>>
>>> On Wed, Jul 25, 2018 at 08:58:42AM +0200, H. Nikolaus Schaller wrote:
>>>> to better match omap3-beagle.dts (which was the basis
>>>> of designing the GTA04).
>>>>
>>>> Signed-off-by: H. Nikolaus Schaller <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/omap3-gta04.dtsi | 14 +++++++-------
>>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
>>>> index 03fe404cbf56..9568e0c4d4bf 100644
>>>> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
>>>> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
>>>> @@ -616,27 +616,27 @@
>>>> interrupt-parent = <&gpmc>;
>>>> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>>>> <1 IRQ_TYPE_NONE>; /* termcount */
>>>> + ti,nand-ecc-opt = "ham1";
>>>> + rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
>>>> nand-bus-width = <16>;
>>>> - ti,nand-ecc-opt = "bch8";
>>>
>>> You are using weeker ECC scheme just to be compatible with another machine?
>>
>> No not another machine. The GTA04 uses the same SoC and NAND chip as the BeagleBoard,
>> so you can imagine GTA04 being a BeagleBoard + a lot of other things.
>>
>> The key reason is to change the ecc scheme is to be compatible with the U-Boot used.
>>
>> BootROM can only handle ham1 for the MLO. And there is no nand-ecc-opt
>> for each partition. So we either can't mix ECC schemes if we want to be able
>> to read/write MLO as the first partition from kernel.
>>
>>> So now you cannot boot already deployed filesystem...
>>
>> No. We always used ham1 and bch8 wasn't working at all here. Therefore nobody
>> did use upstream kernel for NAND yet...
>>
>>> Also is it enough for
>>> NAND chip used?
>>
>> Well, the chip is recommended to use bch8 but BootROM imposes above mentioned limits.
>
> Then common way to handle such a situation is to use 1bit hamming for MLO and BCH8 for
> the rest. You will end with corrupted filesystem with ham1 which I'd consider very
> unfortunate.
>
> (I know there were endless discussions how to handle this situation. It is already
> solved in U-Boot and for updating MLO from Linux I'm using writeloader tool)

Never heard of this tool, but we haven't followed this discussion for years since
we never had problems with "ham1-everywhere". What I am not completely sure is if
ubifs is using its own ECC scheme or not. If it is (like I assume), the user-space
partition is independent of this setting and we just have discussion for MLO/U-Boot
and kernel partitions.

So I think it boils down to the question if
a) upstream should do a suboptimal thing, but be compatible to the current vendor kernel and u-boot
b) upstream should do the right thing, but stay incompatible

Result of b) would e.g. a stock Debian kernel will not be able to read or write the partitions of a device
created with vendor kernel.

And even worse, it is not clear if users will want to erase and reflash the NAND to match what kernel.org defines if they have a running system.

This makes me believe that a) is still better for practical reasons, even if technically worse.

BR,
Nikolaus

>
> ladis
>
>> BR,
>> Nikolaus
>>
>>>
>>> ladis
>>>
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>>
>>>> - gpmc,sync-clk-ps = <0>;
>>>> + gpmc,device-width = <2>;
>>>> gpmc,cs-on-ns = <0>;
>>>> gpmc,cs-rd-off-ns = <44>;
>>>> gpmc,cs-wr-off-ns = <44>;
>>>> gpmc,adv-on-ns = <6>;
>>>> gpmc,adv-rd-off-ns = <34>;
>>>> gpmc,adv-wr-off-ns = <44>;
>>>> - gpmc,we-off-ns = <40>;
>>>> gpmc,oe-off-ns = <54>;
>>>> + gpmc,we-off-ns = <40>;
>>>> gpmc,access-ns = <64>;
>>>> gpmc,rd-cycle-ns = <82>;
>>>> gpmc,wr-cycle-ns = <82>;
>>>> gpmc,wr-access-ns = <40>;
>>>> gpmc,wr-data-mux-bus-ns = <0>;
>>>> - gpmc,device-width = <2>;
>>>> -
>>>> - #address-cells = <1>;
>>>> - #size-cells = <1>;
>>>> + gpmc,sync-clk-ps = <0>;
>>>>
>>>> x-loader@0 {
>>>> label = "X-Loader";
>>>> --
>>>> 2.12.2
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2018-07-25 10:38:59

by Sebastian Reichel

[permalink] [raw]

Subject: Re: [PATCH 04/32] ARM: dts: omap3-gta04: fixes for tvout / venc

Hi,

On Wed, Jul 25, 2018 at 08:58:36AM +0200, H. Nikolaus Schaller wrote:
> port@1 {
> - reg = <1>;
> + reg = <0>;

You should also update the node name.

-- Sebastian

Attachments:

(No filename) (190.00 B)
signature.asc (849.00 B)
Download all attachments

2018-07-25 11:41:52

Hi Sebastian,

> Am 25.07.2018 um 12:37 schrieb Sebastian Reichel <[email protected]>:
>
> Hi,
>
> On Wed, Jul 25, 2018 at 08:58:36AM +0200, H. Nikolaus Schaller wrote:
>> port@1 {
>> - reg = <1>;
>> + reg = <0>;
>
> You should also update the node name.

I did take another look at this and we might have the wrong fix here anyways.
The first indication is that a port@0 already exists soI can't simply rename
the node.

The opa362 is an exceptional dssdev since it has an input and an output port
which should be differentiated by the reg property and the node name.

Ports are apparently matched by reg number, i.e. here:

https://elixir.bootlin.com/linux/v4.18-rc6/source/drivers/gpu/drm/omapdrm/dss/output.c#L146

The issue seems to be that the driver does not set the out->port_num of the endpoint to 1.

https://elixir.bootlin.com/linux/v4.18-rc6/source/drivers/gpu/drm/omapdrm/displays/encoder-opa362.c#L205

So the <1> does not match and we were just lucky to make it working by choosing <0> in the DT.

I'll need a little time to test if a (one-liner) patch for the driver does the same/right thing.

And since it is omapdss related I've put Tomi on CC:

BR,
Nikolaus

Attachments:

signature.asc (817.00 B)
Message signed with OpenPGP using GPGMail

2018-07-25 13:27:10

by Ladislav Michl

[permalink] [raw]

Subject: Re: [PATCH 09/32] ARM: dts: omap3-gta04: make NAND partitions compatible with recent U-Boot

On Wed, Jul 25, 2018 at 02:27:31PM +0200, H. Nikolaus Schaller wrote:
>
> > Am 25.07.2018 um 10:33 schrieb Ladislav Michl <[email protected]>:
> >
> > On Wed, Jul 25, 2018 at 10:18:28AM +0200, H. Nikolaus Schaller wrote:
> >>
> >>> Am 25.07.2018 um 10:07 schrieb Ladislav Michl <[email protected]>:
> >>>
> >>> On Wed, Jul 25, 2018 at 08:58:41AM +0200, H. Nikolaus Schaller wrote:
> >>>> Vendor defined U-Boot has changed the partition scheme a while ago:
> >>>>
> >>>> * kernel partition 6MB
> >>>> * file system partition uses the remainder up to end of the NAND
> >>>> * increased size of the environment partition (to get an OneNAND compatible base address)
> >>>> * shrink the U-Boot partition
> >>>>
> >>>> Let's be compatible (e.g. Debian kernel built from upstream).
> >>>
> >>> That, in fact, is breaking compatibility.
> >>
> >> With what? Nobody is using the old u-boot partition scheme any more
> >> (it is >5 years old).
> >>
> >>> So once you are touching this
> >>> what about relying on partitioning provided by bootloader just to prevent
> >>> something like this happening again?
> >>
> >> Well, we define what compatible means here (since we are the vendor).
> >> And people complain with us. We simply recommend them to upgrade the
> >> boot-loader.
> >
> > Fair enough. Suggestion was to remove partitioning scheme from DTB alltogether
> > and let U-Boot provide one. But you being vendor you decide, of course :)
> > (I'd use only two partitions: MLO and UBI, latter one with BCH8, and store
> > everything in UBI volumes. That's a bit more flexible approach)
>
> Yes, that is a good goal for a future setup and would of course be better.
> Like U-Boot already provides the memory layout for RAM.
>
> Hopefully, someone will work out patches for u-boot plus kernel (which is always
> painful to keep these two in sync and tested). But I don't want to do that now.

At kernel side the only patch needed is to remove partitioning scheme from DTB
and u-boot setup can be merely copied from board/isee/igep00x0
Not having hardware I cannot really test it ;-)

Best regards,
ladis

2018-07-25 17:01:51

by Andreas Kemnade

[permalink] [raw]

Subject: Re: [Letux-kernel] [PATCH 09/32] ARM: dts: omap3-gta04: make NAND partitions compatible with recent U-Boot

On Wed, 25 Jul 2018 10:33:05 +0200
Ladislav Michl <[email protected]> wrote:

> On Wed, Jul 25, 2018 at 10:18:28AM +0200, H. Nikolaus Schaller wrote:
> >
> > > Am 25.07.2018 um 10:07 schrieb Ladislav Michl <[email protected]>:
> > >
> > > On Wed, Jul 25, 2018 at 08:58:41AM +0200, H. Nikolaus Schaller wrote:
> > >> Vendor defined U-Boot has changed the partition scheme a while ago:
> > >>
> > >> * kernel partition 6MB
> > >> * file system partition uses the remainder up to end of the NAND
> > >> * increased size of the environment partition (to get an OneNAND compatible base address)
> > >> * shrink the U-Boot partition
> > >>
> > >> Let's be compatible (e.g. Debian kernel built from upstream).
> > >
> > > That, in fact, is breaking compatibility.
> >
> > With what? Nobody is using the old u-boot partition scheme any more
> > (it is >5 years old).
> >
> > > So once you are touching this
> > > what about relying on partitioning provided by bootloader just to prevent
> > > something like this happening again?
> >
> > Well, we define what compatible means here (since we are the vendor).
> > And people complain with us. We simply recommend them to upgrade the
> > boot-loader.
>
> Fair enough. Suggestion was to remove partitioning scheme from DTB alltogether
> and let U-Boot provide one. But you being vendor you decide, of course :)
> (I'd use only two partitions: MLO and UBI, latter one with BCH8, and store
> everything in UBI volumes. That's a bit more flexible approach)
>
hmm, so using mtdparts kernel commandline parameter? Somehow it sounds
to be sane to not have partition tables in kernel. What only is needed
is to have a nice transition scheme for systems in the wild, can
commandline mtdparts overwrite dtb? So dtb is a fallback?
But I think all that is a future improvement?

Regards,
Andreas

Attachments:

(No filename) (849.00 B)
OpenPGP digital signature

2018-07-25 20:09:28

by Ladislav Michl

[permalink] [raw]

Subject: Re: [Letux-kernel] [PATCH 09/32] ARM: dts: omap3-gta04: make NAND partitions compatible with recent U-Boot

On Wed, Jul 25, 2018 at 06:27:45PM +0200, Andreas Kemnade wrote:
> On Wed, 25 Jul 2018 10:33:05 +0200
> Ladislav Michl <[email protected]> wrote:
>
> > On Wed, Jul 25, 2018 at 10:18:28AM +0200, H. Nikolaus Schaller wrote:
> > >
> > > > Am 25.07.2018 um 10:07 schrieb Ladislav Michl <[email protected]>:
> > > >
> > > > On Wed, Jul 25, 2018 at 08:58:41AM +0200, H. Nikolaus Schaller wrote:
> > > >> Vendor defined U-Boot has changed the partition scheme a while ago:
> > > >>
> > > >> * kernel partition 6MB
> > > >> * file system partition uses the remainder up to end of the NAND
> > > >> * increased size of the environment partition (to get an OneNAND compatible base address)
> > > >> * shrink the U-Boot partition
> > > >>
> > > >> Let's be compatible (e.g. Debian kernel built from upstream).
> > > >
> > > > That, in fact, is breaking compatibility.
> > >
> > > With what? Nobody is using the old u-boot partition scheme any more
> > > (it is >5 years old).
> > >
> > > > So once you are touching this
> > > > what about relying on partitioning provided by bootloader just to prevent
> > > > something like this happening again?
> > >
> > > Well, we define what compatible means here (since we are the vendor).
> > > And people complain with us. We simply recommend them to upgrade the
> > > boot-loader.
> >
> > Fair enough. Suggestion was to remove partitioning scheme from DTB alltogether
> > and let U-Boot provide one. But you being vendor you decide, of course :)
> > (I'd use only two partitions: MLO and UBI, latter one with BCH8, and store
> > everything in UBI volumes. That's a bit more flexible approach)
> >
> hmm, so using mtdparts kernel commandline parameter? Somehow it sounds
> to be sane to not have partition tables in kernel. What only is needed
> is to have a nice transition scheme for systems in the wild, can
> commandline mtdparts overwrite dtb? So dtb is a fallback?

That's beginning to be offtopic here... Anyway, see U-Boot's
CONFIG_FDT_FIXUP_PARTITIONS. Probably better to start a thread on
U-Boot mailing list if needed.

> But I think all that is a future improvement?

Depends on vendor decision, it could be done in a few days :)

Best regards,
ladis