Re: [PATCH] partitions: Align partition numbering with that of Linux kernel

2018-02-16 Thread Sascha Hauer
Hi Andrey,

On Mon, Feb 05, 2018 at 09:18:15AM -0800, Andrey Smirnov wrote:
> Change the way Barebox numbers DOS/EFI partitions, such that it would
> be consistent with how Linux kernel does it. With this change:
> 
>   - Both types of partitions are numbered starting from 1, not 0
> 
>   - Extended DOS partitions always start from 5, leaving first 4 to
> non-extended ones.

I am not very happy with this change because it breaks boards expecting
a certain numbering. See for example:

arch/arm/mach-omap/omap_generic.c:116:static char *envpath = 
"/mnt/mmc0.0/barebox.env";
arch/arm/boards/phytec-som-am335x/defaultenv-physom-am335x/boot/mmc:3:global.bootm.image=/mnt/mmc0.0/linuximage
arch/arm/boards/embedsky-e9/defaultenv-e9/boot/mmc1:3:mount /dev/mmc1.0

There are more examples in-tree and for sure more out of tree.

We need very strong reasons to change this numbering.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH] partitions: Align partition numbering with that of Linux kernel

2018-02-16 Thread Andrey Smirnov
On Mon, Feb 5, 2018 at 9:18 AM, Andrey Smirnov  wrote:
> Change the way Barebox numbers DOS/EFI partitions, such that it would
> be consistent with how Linux kernel does it. With this change:
>
>   - Both types of partitions are numbered starting from 1, not 0
>
>   - Extended DOS partitions always start from 5, leaving first 4 to
> non-extended ones.
>

Sascha, any opinion on this patch?

Thanks,
Andrey Smirnov

> Signed-off-by: Andrey Smirnov 
> ---
>  common/partitions.c| 7 +++
>  common/partitions/dos.c| 2 ++
>  common/partitions/efi.c| 1 +
>  common/partitions/parser.h | 1 +
>  4 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/common/partitions.c b/common/partitions.c
> index 574b31fbb..53d55a49a 100644
> --- a/common/partitions.c
> +++ b/common/partitions.c
> @@ -42,8 +42,7 @@ static LIST_HEAD(partition_parser_list);
>   * @param no Partition number
>   * @return 0 on success
>   */
> -static int register_one_partition(struct block_device *blk,
> -   struct partition *part, int no)
> +static int register_one_partition(struct block_device *blk, struct partition 
> *part)
>  {
> char *partition_name;
> int ret;
> @@ -51,7 +50,7 @@ static int register_one_partition(struct block_device *blk,
> uint64_t size = part->size * SECTOR_SIZE;
> struct cdev *cdev;
>
> -   partition_name = basprintf("%s.%d", blk->cdev.name, no);
> +   partition_name = basprintf("%s.%d", blk->cdev.name, part->number);
> if (!partition_name)
> return -ENOMEM;
> dev_dbg(blk->dev, "Registering partition %s on drive %s\n",
> @@ -150,7 +149,7 @@ int parse_partition_table(struct block_device *blk)
>
> /* at least one partition description found */
> for (i = 0; i < pdesc->used_entries; i++) {
> -   rc = register_one_partition(blk, >parts[i], i);
> +   rc = register_one_partition(blk, >parts[i]);
> if (rc != 0)
> dev_err(blk->dev,
> "Failed to register partition %d on %s 
> (%d)\n",
> diff --git a/common/partitions/dos.c b/common/partitions/dos.c
> index 488c2936f..2ee9b2bd6 100644
> --- a/common/partitions/dos.c
> +++ b/common/partitions/dos.c
> @@ -170,6 +170,7 @@ static void dos_extended_partition(struct block_device 
> *blk, struct partition_de
> get_unaligned_le32([0].partition_start);
> pd->parts[n].size = 
> get_unaligned_le32([0].partition_size);
> pd->parts[n].dos_partition_type = table[0].type;
> +   pd->parts[n].number = partno;
> if (signature)
> sprintf(pd->parts[n].partuuid, "%08x-%02u",
> signature, partno);
> @@ -224,6 +225,7 @@ static void dos_partition(void *buf, struct block_device 
> *blk,
> pd->parts[n].first_sec = pentry.first_sec;
> pd->parts[n].size = pentry.size;
> pd->parts[n].dos_partition_type = 
> pentry.dos_partition_type;
> +   pd->parts[n].number = i + 1;
> if (signature)
> sprintf(pd->parts[n].partuuid, "%08x-%02d",
> signature, i + 1);
> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
> index 88734f166..5dc9e1ad8 100644
> --- a/common/partitions/efi.c
> +++ b/common/partitions/efi.c
> @@ -456,6 +456,7 @@ static void efi_partition(void *buf, struct block_device 
> *blk,
> pentry = >parts[pd->used_entries];
> pentry->first_sec = le64_to_cpu(ptes[i].starting_lba);
> pentry->size = le64_to_cpu(ptes[i].ending_lba) - 
> pentry->first_sec;
> +   pentry->number = pd->used_entries + 1;
> pentry->size++;
> part_set_efi_name([i], pentry->name);
> snprintf(pentry->partuuid, sizeof(pentry->partuuid), "%pUl", 
> [i].unique_partition_guid);
> diff --git a/common/partitions/parser.h b/common/partitions/parser.h
> index 8ad134a9a..713933794 100644
> --- a/common/partitions/parser.h
> +++ b/common/partitions/parser.h
> @@ -20,6 +20,7 @@ struct partition {
> char partuuid[MAX_PARTUUID_STR];
> uint64_t first_sec;
> uint64_t size;
> +   int number;
>  };
>
>  struct partition_desc {
> --
> 2.14.3
>

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH 4/4] commands: version: Add framebuffer output support

2018-02-16 Thread Andrey Smirnov
On Thu, Feb 8, 2018 at 11:29 PM, Sascha Hauer  wrote:
> Hi Andrey,
>
> On Mon, Feb 05, 2018 at 09:29:35AM -0800, Andrey Smirnov wrote:
>> Extend "version" command to be capable of rendeing version information
>> on framebuffer devices.
>>
>> Signed-off-by: Andrey Smirnov 
>> ---
>>  commands/Kconfig   |   8 +++
>>  commands/version.c | 198 
>> -
>>  2 files changed, 204 insertions(+), 2 deletions(-)
>>
>> diff --git a/commands/Kconfig b/commands/Kconfig
>> index 095536849..1cad5d608 100644
>> --- a/commands/Kconfig
>> +++ b/commands/Kconfig
>> @@ -238,6 +238,14 @@ config CMD_VERSION
>>
>> barebox 2014.05.0-00142-gb289373 #177 Mon May 12 20:35:55 CEST 2014
>>
>> +config CMD_VERSION_OUTPUT_TO_FRAMEBUFFER
>> +   bool
>> +   depends on CMD_VERSION
>> +   prompt "support displaying version via framebuffer"
>> +   help
>> + Selecting this option will enable version command to output
>> + version information onto display backed by a frambuffer
>> +
>>  config CMD_MMC_EXTCSD
>>   tristate
>>   prompt "read/write eMMC ext. CSD register"
>> diff --git a/commands/version.c b/commands/version.c
>> index 090f2dd13..ba74a993c 100644
>> --- a/commands/version.c
>> +++ b/commands/version.c
>> @@ -20,16 +20,210 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define FRAME_SIZE   8
>> +
>> +struct placement {
>> + int x, y;
>> +};
>> +
>> +typedef struct placement placement_function_t (struct screen *, int, int);
>> +
>> +struct placement
>> +placement_center(struct screen *sc, int text_width, int text_height)
>> +{
>> + struct placement p;
>> +
>> + p.x = (sc->info->xres - text_width) / 2;
>> + p.y = (sc->info->yres - text_height) / 2;
>> +
>> + return p;
>> +}
>> +
>> +struct placement
>> +placement_upper_left(struct screen *sc, int text_width, int text_height)
>> +{
>> + struct placement p;
>> +
>> + p.x = FRAME_SIZE;
>> + p.y = FRAME_SIZE;
>> +
>> + return p;
>> +}
>> +
>> +struct placement
>> +placement_upper_right(struct screen *sc, int text_width, int text_height)
>> +{
>> + struct placement p;
>> +
>> + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE;
>> + p.y = FRAME_SIZE;
>> +
>> + return p;
>> +}
>> +
>> +struct placement
>> +placement_lower_left(struct screen *sc, int text_width, int text_height)
>> +{
>> + struct placement p;
>> +
>> + p.x = FRAME_SIZE;
>> + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE;
>> +
>> + return p;
>> +}
>> +
>> +struct placement
>> +placement_lower_right(struct screen *sc, int text_width, int text_height)
>> +{
>> + struct placement p;
>> +
>> + p.x = sc->info->xres - 1 - text_width  - FRAME_SIZE;
>> + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE;
>> +
>> + return p;
>> +}
>>
>>  static int do_version(int argc, char *argv[])
>>  {
>> - printf ("\n%s\n", version_string);
>> + const struct font_desc *font;
>> + const char *fbdev = NULL;
>> + struct screen *sc;
>> + u8 fg[3], bg[3];
>> + int text_width;
>> + int text_height;
>> +
>> + placement_function_t *place;
>> + struct placement placement;
>> +
>> + if (IS_ENABLED(CONFIG_CMD_VERSION_OUTPUT_TO_FRAMEBUFFER)) {
>
> This is quite much code added to the version command, given that the
> command previously only was a one-liner. I think this should rather be a
> separate command. This could depend on framebuffer and other needed
> stuff which would make the first two patches unnecessary. I think
> stubbing away graphics functions when the user is graphics-only code is
> a bit over the top.
>

OK, will change in v2.

> The placement functions look as if they should be moved to some font
> handling generic code.

OK, will do.

>
> Also it's always nice to have a C API for anything we also have as
> command. It has proven to be useful so many times.
>

Sure, will do.

>> +BAREBOX_CMD_HELP_OPT ("-f color\t", "foreground color, in hex RRGGBB 
>> format")
>> +BAREBOX_CMD_HELP_OPT ("-b color\t", "background color, in hex RRGGBB 
>> format")
>
> I'm not sure how useful these are as parameters to a command. How about
> a nv variable? This could be used by the framebuffer console aswell.
>

Sure, sounds reasonable.

Thanks,
Andrey Smirnov

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: barebox for a ti am335x based board

2018-02-16 Thread Giorgio Dal Molin
Hi,

thank you for the answer, meanwhile I also found the bootinfo
details in the ~5000 pages am335x datasheet.

Starting the second stage bootloader (barebox.bin file on the mmc)
from the MLO also works ('out of the box' actually).

giorgio

> On February 16, 2018 at 8:51 AM Sascha Hauer  wrote:
> 
> 
> Hi Giorgio,
> 
> On Tue, Feb 13, 2018 at 11:12:54PM +0100, Giorgio Dal Molin wrote:
> > Hi all,
> > 
> > I'm trying to add board code for an am335x based HW project.
> > I had a look at similar boards like 'boards/phytec-som-am335x'
> > or 'boards/beaglebone'.
> > I understand that I need two images: the (little) MLO and the
> > (bigger) barebox.
> > 
> > What it's not really clear is how the MLO loads and starts the
> > barebox.
> 
> See arch/arm/mach-omap/xload.c. It is compiled when no shell is enabled
> (I think at least for clarity there should be a separate Kconfig
> symbol). In this file omap_xload() detects where the SoC has booted from
> and loads the full barebox from the same source.
> 
> > 
> > Moreover I saw there's always a call to:
> > 
> >  am33xx_save_bootinfo((void *)bootinfo);
> > 
> > in the MLO lowlevel entry function; I suspect the argument
> > 'bootinfo' is the register r0 left by the function __barebox_arm_head()
> > but I cannot really understand the logic behind it.
> 
> When the am335x ROM passes control to the bootloader then it passes in
> R0 a pointer to the bootinfo. This is a three word structure which has
> informations where the SoC has booted from. In am33xx_save_bootinfo() we
> copy the information to a known place where we can pick it up later, see
> am33xx_bootsource().
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.   | |
> Industrial Linux Solutions | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


re-generate the builtin environment

2018-02-16 Thread Giorgio Dal Molin
Hi,

I want to build a default environment in the barebox for my
board.


For this I defined the following vars in the .config file:

...
CONFIG_DEFAULT_ENVIRONMENT=y
CONFIG_DEFAULT_COMPRESSION_LZ4=y
CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU=y
CONFIG_DEFAULT_ENVIRONMENT_PATH="arch/arm/boards/am335x-evm/defaultenv-am335x_evm"
...

What I observe is that the first time I build barebox the default env is also 
built;
but after that, if I modify something under CONFIG_DEFAULT_ENVIRONMENT_PATH and
just rebuild the barebox dir. without a 'make clean' then the changes are 
missed.

It this correct or do I do something wrong.

The second question is: I noticed that in the board Makefile:

 arch/arm/boards/am335x-evm/Makefile

one can add the definition:

bbenv-y += defaultenv-am335x_evm

Isn't it a duplicate of the CONFIG_DEFAULT_ENVIRONMENT_PATH make var. ?

giorgio

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH 3/3] drivers: mtd: nand: omap: Return stat value

2018-02-16 Thread Sascha Hauer
On Thu, Feb 15, 2018 at 01:51:26PM +0100, Daniel Schultz wrote:
> Hi Sascha,
> 
> 
> On 01/30/2018 08:11 AM, Sascha Hauer wrote:
> > On Mon, Jan 29, 2018 at 02:04:11PM +0100, Daniel Schultz wrote:
> > > The read page function should return the total count of flipped bits,
> > > otherwise the caller always thinks no bitflip occured.
> > > 
> > > Signed-off-by: Daniel Schultz 
> > > ---
> > >   drivers/mtd/nand/nand_omap_gpmc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/mtd/nand/nand_omap_gpmc.c 
> > > b/drivers/mtd/nand/nand_omap_gpmc.c
> > > index e18ce63..38f4960 100644
> > > --- a/drivers/mtd/nand/nand_omap_gpmc.c
> > > +++ b/drivers/mtd/nand/nand_omap_gpmc.c
> > > @@ -712,7 +712,7 @@ static int omap_gpmc_read_page_bch_rom_mode(struct 
> > > mtd_info *mtd,
> > >   else
> > >   mtd->ecc_stats.corrected += stat;
> > > - return 0;
> > > + return stat;
> > >   }
> > I'm afraid this is not enough. read_page should return the maximum
> > number of bitflips in any ECC step. You first have to change
> > omap_correct_bch() so that it returns this number.
> ahh, we worked on this problem a half year ago and it seems like three
> patches are missing upstream:
> 
> http://lists.infradead.org/pipermail/barebox/2017-June/030385.html
> http://lists.infradead.org/pipermail/barebox/2017-June/030384.html
> http://lists.infradead.org/pipermail/barebox/2017-June/030355.html

Oh, I see. It seems I have either forgotten to merge them or I have
waited for feedback. Could you create a series for all missing patches
including your new patch and resend it, provided it works as expected of
course?

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox