Re: [PATCH v2 11/13] ARM: boards: sunxi: Add initial support for the pinephone
Hi Sascha,
Thanks for your review
On Tue, May 30, 2023 at 10:42:36AM +0200, Sascha Hauer wrote:
> On Thu, May 25, 2023 at 01:43:26AM +0200, Jules Maselbas wrote:
> > Signed-off-by: Jules Maselbas
> > ---
> > arch/arm/boards/Makefile| 1 +
> > arch/arm/boards/pine64-pinephone/Makefile | 2 +
> > arch/arm/boards/pine64-pinephone/board.c| 0
> > arch/arm/boards/pine64-pinephone/lowlevel.c | 104
> > arch/arm/configs/pinephone_defconfig| 12 +++
> > arch/arm/dts/Makefile | 1 +
> > arch/arm/dts/sun50i-a64-pinephone-1_2.dts | 3 +
> > arch/arm/mach-sunxi/Kconfig | 17
> > images/Makefile.sunxi | 9 ++
> > include/mach/sunxi/init.h | 4 +
> > 10 files changed, 153 insertions(+)
> > create mode 100644 arch/arm/boards/pine64-pinephone/Makefile
> > create mode 100644 arch/arm/boards/pine64-pinephone/board.c
> > create mode 100644 arch/arm/boards/pine64-pinephone/lowlevel.c
> > create mode 100644 arch/arm/configs/pinephone_defconfig
> > create mode 100644 arch/arm/dts/sun50i-a64-pinephone-1_2.dts
> >
> > diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> > index b204c257f6..f4796f5374 100644
> > --- a/arch/arm/boards/Makefile
> > +++ b/arch/arm/boards/Makefile
> > @@ -96,6 +96,7 @@ obj-$(CONFIG_MACH_PHYTEC_SOM_IMX6)+=
> > phytec-som-imx6/
> > obj-$(CONFIG_MACH_PHYTEC_PHYCORE_IMX7) += phytec-phycore-imx7/
> > obj-$(CONFIG_MACH_PHYTEC_PHYCORE_STM32MP1) += phytec-phycore-stm32mp1/
> > obj-$(CONFIG_MACH_PHYTEC_SOM_IMX8MQ) += phytec-som-imx8mq/
> > +obj-$(CONFIG_MACH_PINE64_PINEPHONE)+= pine64-pinephone/
> > obj-$(CONFIG_MACH_PLATHOME_OPENBLOCKS_AX3) += plathome-openblocks-ax3/
> > obj-$(CONFIG_MACH_PLATHOME_OPENBLOCKS_A6) += plathome-openblocks-a6/
> > obj-$(CONFIG_MACH_PM9261) += pm9261/
> > diff --git a/arch/arm/boards/pine64-pinephone/Makefile
> > b/arch/arm/boards/pine64-pinephone/Makefile
> > new file mode 100644
> > index 00..092c31d6b2
> > --- /dev/null
> > +++ b/arch/arm/boards/pine64-pinephone/Makefile
> > @@ -0,0 +1,2 @@
> > +lwl-y += lowlevel.o
> > +obj-y += board.o
> > diff --git a/arch/arm/boards/pine64-pinephone/board.c
> > b/arch/arm/boards/pine64-pinephone/board.c
> > new file mode 100644
> > index 00..e69de29bb2
> > diff --git a/arch/arm/boards/pine64-pinephone/lowlevel.c
> > b/arch/arm/boards/pine64-pinephone/lowlevel.c
> > new file mode 100644
> > index 00..262d194864
> > --- /dev/null
> > +++ b/arch/arm/boards/pine64-pinephone/lowlevel.c
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> This file is missing in this series. Forgot to git add it?
ooops, yes forgot to add it.
There is almost nothing in it:
```
#include
#define SUN50I_A64_ENTRY_FUNCTION(name, arg0, arg1, arg2) \
ENTRY_FUNCTION_WITHSTACK(name, SUN50I_PBL_STACK_TOP, arg0, arg1, arg2)
```
that's all
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#ifdef DEBUG
> > +static void debug_led_rgb(int rgb)
> > +{
> > + void __iomem *piobase = SUN50I_PIO_BASE_ADDR;
> > + uint32_t clr, set = 0;
> > + int r = rgb & 0xff;
> > + int g = rgb & 0x00ff00;
> > + int b = rgb & 0xff;
> > +
> > + clr = (1 << 19) | (1 << 18) | (1 << 20);
> > + set |= r ? 1 << 19 : 0;
> > + set |= g ? 1 << 18 : 0;
> > + set |= b ? 1 << 20 : 0;
> > +
> > + clrsetbits_le32(piobase + PIO_PD_DATA, clr, set);
> > +}
> > +
> > +static void debug_led_init(void)
> > +{
> > + void __iomem *ccubase = SUN50I_CCU_BASE_ADDR;
> > + void __iomem *piobase = SUN50I_PIO_BASE_ADDR;
> > +
> > + /* PIO clock enable */
> > + setbits_le32(ccubase + CCU_BUS_CLK_GATE2, BIT(5));
> > + /* LED set output */
> > + clrsetbits_le32(piobase + PIO_PD_CFG2, 0x000fff00, 0x00011100);
> > +}
> > +#else
> > +static void debug_led_rgb(int rgb) {}
> > +static void debug_led_init(void) {}
> > +#endif
> > +
> > +SUN50I_A64_ENTRY_FUNCTION(start_pine64_pinephone, r0, r1, r2)
> > +{
> > + extern char __dtb_z_sun50i_a64_pinephone_1_2_start[];
> > + void *fdt;
> > + u32 size;
> > +
> > + sunxi_switch_to_aarch64(.text_head_soc_header2, SUN50I_A64_RVBAR_IOMAP);
> > +
> > + debug_led_init();
> > + debug_led_rgb(0x00);
> > +
> > + sun50i_cpu_lowlevel_init();
> > + sun50i_uart_setup();
> > +
> > + relocate_to_current_adr();
> > + setup_c();
> > +
> > + /* Skip SDRAM initialization if we run from it */
> > + if (get_pc() < SUN50I_DRAM_ADDR) {
> > + size = sun50i_a64_lpddr3_dram_init();
> > + if (size == 0) {
> > + puts_ll("FAIL: dram init\r\n");
> > + goto reset;
> > + }
> > + puthex_ll(size);
> > + putc_ll('\r'); putc_ll('\n');
> > +
Re: [PATCH 18/18] state: allow lookup of barebox state partition by Type GUID
On 31.05.23 22:01, Marco Felsch wrote:
> Hi Ahmad,
>
> On 23-05-31, Ahmad Fatoum wrote:
>> The backend device tree property so far always pointed at a partition.
>> Let's allow pointing it at GPT storage devices directly and lookup
>> the correct barebox state partition by the well-known type GUID:
>>
>> 4778ed65-bf42-45fa-9c5b-287a1dc4aab1
>
> we should add an example within the Documetation/ too.
I'd wait until we have a new dt-utils release with the same
binding. Anything else would be confusing.
>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> common/state/state.c | 22 ++
>> include/driver.h | 17 +
>> include/state.h | 4
>> 3 files changed, 43 insertions(+)
>>
>> diff --git a/common/state/state.c b/common/state/state.c
>> index 88e246198fb8..8f56c60b0e82 100644
>> --- a/common/state/state.c
>> +++ b/common/state/state.c
>> @@ -21,8 +21,10 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -595,6 +597,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t
>> *offset, size_t *size)
>> }
>> #endif
>>
>> +static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
>> +
>> /*
>> * state_new_from_node - create a new state instance from a device_node
>> *
>> @@ -641,6 +645,24 @@ struct state *state_new_from_node(struct device_node
>> *node, bool readonly)
>> goto out_release_state;
>> }
>>
>> +/* Is the backend referencing an on-disk partitonable block device? */
>> +if (cdev_is_block_disk(cdev)) {
>> +struct cdev *partcdev = NULL;
>> +
>> +if (cdev_is_gpt_partitioned(cdev))
>> +partcdev = cdev_find_child_by_typeuuid(cdev,
>> _state_partition_guid);
>> +
>> +if (!partcdev) {
>> +ret = -EINVAL;
>> +goto out_release_state;
>> +}
>> +
>> +pr_debug("%s: backend GPT partition looked up via
>> PartitionTypeGUID\n",
>> + node->full_name);
>> +
>> +cdev = partcdev;
>> +}
>
> What about having the above logic within a seperate function and the
> above code would be:
>
> if (cdev_is_block_disk(cdev))
> cdev = get_cdev_by_typeuuid(cdev, _state_partition_guid)
>
> if (!cdev) {
> ret = -EINVAL;
> goto out_release_state;
> }
>
> This way we would have everything in place to re-use the same logic for
> the barebox-environmnet too. What do you think?
cdev from of_cdev_find not being available -> -EPROBE_DEFER
cdev GPT partition not existing,despite backend pointing at root device ->
-EINVAL
So the cdev check needs to be within if (cdev_is_block_disk(cdev)).
I don't object to combining cdev_is_gpt_partitioned and
cdev_find_child_by_typeuuid into
cdev_find_child_by_gpt_typeuuid though.
Thanks for all the review!
Ahmad
>
> Regards,
> Marco
>
>> +
>> state->backend_path = cdev_to_devpath(cdev, , );
>>
>> pr_debug("%s: backend resolved to %s\n", node->full_name,
>> state->backend_path);
>> diff --git a/include/driver.h b/include/driver.h
>> index 6407f7d6ba36..579b03fbac34 100644
>> --- a/include/driver.h
>> +++ b/include/driver.h
>> @@ -585,6 +585,23 @@ extern struct list_head cdev_list;
>> #define for_each_cdev(cdev) \
>> list_for_each_entry(cdev, _list, list)
>>
>> +#define for_each_cdev_partition(partcdev, cdev) \
>> +list_for_each_entry((partcdev), &(cdev)->partitions, partition_entry)
>> +
>> +
>> +static inline struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev,
>> + guid_t *typeuuid)
>> +{
>> +struct cdev *partcdev;
>> +
>> +for_each_cdev_partition(partcdev, cdev) {
>> +if (guid_equal(>typeuuid, typeuuid))
>> +return partcdev;
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> #define DEVFS_PARTITION_FIXED (1U << 0)
>> #define DEVFS_PARTITION_READONLY(1U << 1)
>> #define DEVFS_IS_CHARACTER_DEV (1U << 3)
>> diff --git a/include/state.h b/include/state.h
>> index bffcd5a9007f..3daf82c0735f 100644
>> --- a/include/state.h
>> +++ b/include/state.h
>> @@ -62,4 +62,8 @@ static inline int state_read_mac(struct state *state,
>> const char *name, u8 *buf)
>>
>> #endif /* #if IS_ENABLED(CONFIG_STATE) / #else */
>>
>> +#define BAREBOX_STATE_PARTITION_GUID \
>> +GUID_INIT(0x4778ed65, 0xbf42, 0x45fa, 0x9c, 0x5b, \
>> + 0x28, 0x7a, 0x1d, 0xc4, 0xaa, 0xb1)
>> +
>> #endif /* __STATE_H */
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 16/18] cdev: use cdev::dos_partition_type only if cdev_is_mbr_partitioned
On 31.05.23 20:54, Marco Felsch wrote:
> On 23-05-31, Ahmad Fatoum wrote:
>> dos_partition_type == 0 can mean that either a partition is not
>> a MBR partition or that it indeed has a partition type of 0x00.
>>
>> In preparation for using that field in a union, explicitly check if we
>> have a MBR partition.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> common/blspec.c | 2 +-
>> fs/fs.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/blspec.c b/common/blspec.c
>> index e95a8dba8d76..8c7970da8915 100644
>> --- a/common/blspec.c
>> +++ b/common/blspec.c
>> @@ -729,7 +729,7 @@ int blspec_scan_device(struct bootentries *bootentries,
>> struct device *dev)
>> * partition with the MBR type id of 0xEA already exists it
>> * should be used as $BOOT
>> */
>> -if (cdev->dos_partition_type == 0xea) {
>> +if (cdev_is_mbr_partitioned(cdev->master) &&
>> cdev->dos_partition_type == 0xea) {
>
> Since you already various helpers to drop priv direct access, what
> about:
>
> if (cdev_dos_partition_type(cdev) == 0xea)
>
> Within the helper you can check for the cdev_is_mbr_partitioned().
We only have a single call site and there'll be a separate series that adds:
if (cdev_is_gpt_partitioned(cdev->master) && guid_equal(>typeuuid,
blspec_xbootldr_guid))
/* use that */;
after this line. So I'd rather leave it as is.
Thanks,
Ahmad
>
> ?
>
> Regards,
> Marco
>
>> ret = blspec_scan_cdev(bootentries, cdev);
>> if (ret == 0)
>> ret = -ENOENT;
>> diff --git a/fs/fs.c b/fs/fs.c
>> index 2d2d327c5fbc..9a92e6e251e5 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -108,7 +108,7 @@ void cdev_print(const struct cdev *cdev)
>>
>> if (cdev->filetype)
>> nbytes += printf("Filetype: %s\t",
>> file_type_to_string(cdev->filetype));
>> -if (cdev->dos_partition_type)
>> +if (cdev_is_mbr_partitioned(cdev->master))
>> nbytes += printf("DOS parttype: 0x%02x\t",
>> cdev->dos_partition_type);
>> if (*cdev->partuuid || *cdev->diskuuid)
>> nbytes += printf("%sUUID: %s", cdev_is_partition(cdev) ? "PART"
>> : "DISK",
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 15/18] state: factor device path lookup into helper function
On 31.05.23 19:54, Marco Felsch wrote:
> Hi Ahmad,
>
> On 23-05-31, Ahmad Fatoum wrote:
>> The #ifdef __BAREBOX__ is meant for easier synchronization with
>> dt-utils. We'll keep that intact, but move it out of the function to not
>> break reading flow. After sync, dt-utils would now need to implement
>>
>> of_cdev_find
>> cdev_to_devpath
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> common/state/state.c | 30 +++---
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/state/state.c b/common/state/state.c
>> index 11cc86ff73be..88e246198fb8 100644
>> --- a/common/state/state.c
>> +++ b/common/state/state.c
>> @@ -581,6 +581,20 @@ void state_release(struct state *state)
>> free(state);
>> }
>>
>> +#ifdef __BAREBOX__
>> +static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
>> +{
>> +/*
>> + * We only accept partitions exactly mapping the barebox-state,
>> + * but dt-utils may need to set non-zero values here
>> + */
>> +*offset = 0;
>> +*size = 0;
>> +
>> +return basprintf("/dev/%s", cdev->name);
>> +}
>> +#endif
>
> We could get rid of the #ifdef if we move this function to some barebox
> internal code not shared with dt-utils.
Setting offset and size to zero makes no sense elsewhere, that's why I left it
here.
> Regards,
> Marco
>
>> +
>> /*
>> * state_new_from_node - create a new state instance from a device_node
>> *
>> @@ -597,8 +611,9 @@ struct state *state_new_from_node(struct device_node
>> *node, bool readonly)
>> const char *alias;
>> uint32_t stridesize;
>> struct device_node *partition_node;
>> -off_t offset = 0;
>> -size_t size = 0;
>> +struct cdev *cdev;
>> +off_t offset;
>> +size_t size;
>>
>> alias = of_alias_get(node);
>> if (!alias) {
>> @@ -617,11 +632,8 @@ struct state *state_new_from_node(struct device_node
>> *node, bool readonly)
>> goto out_release_state;
>> }
>>
>> -#ifdef __BAREBOX__
>> -ret = of_find_path_by_node(partition_node, >backend_path, 0);
>> -#else
>> -ret = of_get_devicepath(partition_node, >backend_path, ,
>> );
>> -#endif
>> +cdev = of_cdev_find(partition_node);
>> +ret = PTR_ERR_OR_ZERO(cdev);
>> if (ret) {
>> if (ret != -EPROBE_DEFER)
>> dev_err(>dev, "state failed to parse path to
>> backend: %s\n",
>> @@ -629,6 +641,10 @@ struct state *state_new_from_node(struct device_node
>> *node, bool readonly)
>> goto out_release_state;
>> }
>>
>> +state->backend_path = cdev_to_devpath(cdev, , );
>> +
>> +pr_debug("%s: backend resolved to %s\n", node->full_name,
>> state->backend_path);
>> +
>> state->backend_reproducible_name =
>> of_get_reproducible_name(partition_node);
>>
>> ret = of_property_read_string(node, "backend-type", _type);
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 12/18] common: partitions: record whether disk is GPT or MBR partitioned
On 31.05.23 19:33, Marco Felsch wrote:
> On 23-05-31, Ahmad Fatoum wrote:
>> Currently, the only way to differentiate between a GPT disk and a MBR
>> one is to check whether the cdev's device has a guid (=> GPT) or a
>> nt_signature (=> MBR) device parameter. We already have a flag parameter
>> though, so let's record this info there for easy retrieval.
>>
>> We intentionally don't use the struct cdev::filetype member, because
>> we don't want to change behavior of file_detect_type().
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> common/partitions/dos.c | 2 ++
>> common/partitions/efi.c | 2 ++
>> fs/fs.c | 4
>> include/driver.h| 20 +---
>> 4 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/partitions/dos.c b/common/partitions/dos.c
>> index ad60c0b27b46..7472824b00b9 100644
>> --- a/common/partitions/dos.c
>> +++ b/common/partitions/dos.c
>> @@ -185,6 +185,8 @@ static void dos_partition(void *buf, struct block_device
>> *blk,
>> if (signature)
>> sprintf(blk->cdev.diskuuid, "%08x", signature);
>>
>> +blk->cdev.flags |= DEVFS_IS_MBR_PARTITIONED;
>> +
>> table = (struct partition_entry *)[446];
>>
>> for (i = 0; i < 4; i++) {
>> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
>> index 780a8695e8a8..df63b82afe24 100644
>> --- a/common/partitions/efi.c
>> +++ b/common/partitions/efi.c
>> @@ -449,6 +449,8 @@ static void efi_partition(void *buf, struct block_device
>> *blk,
>> snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl",
>> >disk_guid);
>> dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
>>
>> +blk->cdev.flags |= DEVFS_IS_GPT_PARTITIONED;
>> +
>> nb_part = le32_to_cpu(gpt->num_partition_entries);
>>
>> if (nb_part > MAX_PARTITION) {
>> diff --git a/fs/fs.c b/fs/fs.c
>> index 9d8aab268ca4..2d2d327c5fbc 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -94,6 +94,10 @@ void cdev_print(const struct cdev *cdev)
>> printf(" table-partition");
>> if (cdev->flags & DEVFS_IS_MCI_MAIN_PART_DEV)
>> printf(" mci-main-partition");
>> +if (cdev->flags & DEVFS_IS_GPT_PARTITIONED)
>> +printf(" gpt-partitioned");
>> +if (cdev->flags & DEVFS_IS_MBR_PARTITIONED)
>> +printf(" mbr-partitioned");
>> if (cdev->mtd)
>> printf(" mtd");
>> printf(" )");
>> diff --git a/include/driver.h b/include/driver.h
>> index 118d2adb6750..5f2eae65466f 100644
>> --- a/include/driver.h
>> +++ b/include/driver.h
>> @@ -584,9 +584,23 @@ extern struct list_head cdev_list;
>> #define DEVFS_PARTITION_FIXED (1U << 0)
>> #define DEVFS_PARTITION_READONLY(1U << 1)
>> #define DEVFS_IS_CHARACTER_DEV (1U << 3)
>> -#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 4)
>> -#define DEVFS_PARTITION_FROM_OF (1U << 5)
>> -#define DEVFS_PARTITION_FROM_TABLE (1U << 6)
>> +#define DEVFS_PARTITION_FROM_OF (1U << 4)
>> +#define DEVFS_PARTITION_FROM_TABLE (1U << 5)
>> +#define DEVFS_IS_GPT_PARTITIONED(1U << 6)
>> +#define DEVFS_IS_MBR_PARTITIONED(1U << 7)
>> +#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 8)
>
> Why do you reorder the bits here again?
I wanted the partition bits to be together.
>> +
>> +static inline bool cdev_is_gpt_partitioned(const struct cdev *master)
>> +{
>> +return master && (master->flags & DEVFS_IS_GPT_PARTITIONED)
>> +== DEVFS_IS_GPT_PARTITIONED;
>
> Why not just: 'return !!(master->flags & DEVFS_IS_GPT_PARTITIONED)' ?
Left over from when this was more than one bit. I will change to:
return master && (master->flags & DEVFS_IS_GPT_PARTITIONED)
I want to keep the NULL check, as we only set this for the master device.
Cheers,
Ahmad
>
> Regards,
> Marco
>
>> +}
>> +
>> +static inline bool cdev_is_mbr_partitioned(const struct cdev *master)
>> +{
>> +return master && (master->flags & DEVFS_IS_MBR_PARTITIONED)
>> +== DEVFS_IS_MBR_PARTITIONED;
>> +}
>>
>> struct cdev *devfs_add_partition(const char *devname, loff_t offset,
>> loff_t size, unsigned int flags, const char *name);
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 10/18] cdev: have devfs_add_partition return existing identical partition, not NULL
Hello Marco,
On 31.05.23 19:23, Marco Felsch wrote:
> Hi Ahmad,
>
> On 23-05-31, Ahmad Fatoum wrote:
>> Starting with commit 7f9f45b9bfef ("devfs: Do not create overlapping
>> partitions"), any overlapping is disallowed. Overlapping can be useful
>> though to bridge the gap between partition described in DT and via
>> on-disk partition tables. Let's handle the case of identical partitions
>> specially and have it neither be an error or a duplicate partition, but
>> instead just return the existing partition. This existing partition will
>> be given a device tree node and thus enabling schemes like:
>>
>> &{/state} {
>> backend = <_part>;
>> };
>>
>>{
>> partitions {
>> compatible = "fixed-partitions";
>> #address-cells = <2>;
>> #size-cells = <2>;
>>
>> state_part: partition@530 {
>> label = "barebox-state";
>> /* will be folded with overlapping GPT partition if
>> found */
>> reg = <0x0 0x530 0x0 0x10>;
>> };
>> };
>> };
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> fs/devfs-core.c | 50 ++---
>> 1 file changed, 39 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
>> index a0732dafca42..b3a274d01ee0 100644
>> --- a/fs/devfs-core.c
>> +++ b/fs/devfs-core.c
>> @@ -402,6 +402,12 @@ int devfs_remove(struct cdev *cdev)
>> return 0;
>> }
>>
>> +static bool region_identical(loff_t starta, loff_t lena,
>> + loff_t startb, loff_t lenb)
>> +{
>> +return starta == startb && lena == lenb;
>> +}
>> +
>> static bool region_overlap(loff_t starta, loff_t lena,
>> loff_t startb, loff_t lenb)
>> {
>> @@ -412,10 +418,22 @@ static bool region_overlap(loff_t starta, loff_t lena,
>> return 1;
>> }
>>
>> -static int check_overlap(struct cdev *cdev, const char *name, loff_t
>> offset, loff_t size)
>> +/**
>> + * check_overlap() - check overlap with existing partitions
>> + * @cdev: parent cdev
>> + * @name: partition name for informational purposes on conflict
>> + * @offset: offset of new partition to be added
>> + * @size: size of new partition to be added
>> + *
>> + * Return: NULL if no overlapping partition found or overlapping
>> + * partition if and only if it's identical in offset and size
>> + * to an existing partition. Otherwise, PTR_ERR(-EINVAL).
>> + */
>> +static struct cdev *check_overlap(struct cdev *cdev, const char *name,
>> loff_t offset, loff_t size)
>> {
>> struct cdev *cpart;
>> loff_t cpart_offset;
>> +int ret;
>>
>> list_for_each_entry(cpart, >partitions, partition_entry) {
>> cpart_offset = cpart->offset;
>> @@ -428,20 +446,28 @@ static int check_overlap(struct cdev *cdev, const char
>> *name, loff_t offset, lof
>> if (cpart->mtd)
>> cpart_offset = cpart->mtd->master_offset;
>>
>> -if (region_overlap(cpart_offset, cpart->size,
>> - offset, size))
>> +if (region_identical(cpart_offset, cpart->size, offset, size)) {
>> +ret = 0;
>> goto conflict;
>> +}
>
> The 'goto conflict' is a bit misleading here since this is no conflict
> as you described within the commit message.
It's still a conflict, but one that can be resolved by returning the existing
partition.
> I would rather do:
>
> if (region_identical(cpart_offset, cpart->size, offset, size))
> goto out_identical;
>
> and replace the __pr_printk() by pr_debug(). This way you split
> __pr_printk() and drop the ternary operator. The rest lgtm.
The print line is going to be long anyway, so why not share it between
the two cases?
>
> Regards,
> Marco
>
>> +
>> +if (region_overlap(cpart_offset, cpart->size, offset, size)) {
>> +ret = -EINVAL;
>> +goto conflict;
>> +}
>> }
>>
>> -return 0;
>> +return NULL;
>>
>> conflict:
>> -pr_err("New partition %s (0x%08llx-0x%08llx) on %s "
>> -"overlaps with partition %s (0x%08llx-0x%08llx), not creating
>> it\n",
>> -name, offset, offset + size - 1, cdev->name,
>> -cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
>> +__pr_printk(ret ? MSG_WARNING : MSG_DEBUG,
>> +"New partition %s (0x%08llx-0x%08llx) on %s "
>> +"%s with partition %s (0x%08llx-0x%08llx), not creating
>> it\n",
>> +name, offset, offset + size - 1, cdev->name,
>> +ret ? "conflicts" : "identical",
>> +cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
>>
>> -return -EINVAL;
>> +return ret ? ERR_PTR(ret) : cpart;
>> }
>>
>> static
Re: [PATCH 04/18] of: partition: support of_partition_ensure_probed on parent device
Hello Marco,
On 31.05.23 18:30, Marco Felsch wrote:
> Hi Ahmad,
>
> On 23-05-31, Ahmad Fatoum wrote:
>> barebox-state code uses of_partition_ensure_probed to resolve the
>> backend property. We want to allow backend to point directly at a
>> storage device instead of a partition. We can't determine whether a DT
>> device is a storage device though before it's probed, so let's have
>> of_partition_ensure_probed support either case.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> drivers/of/partition.c | 26 ++
>> drivers/of/platform.c | 2 +-
>> 2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/of/partition.c b/drivers/of/partition.c
>> index 40c47f554ad2..a70e503cec9e 100644
>> --- a/drivers/of/partition.c
>> +++ b/drivers/of/partition.c
>> @@ -110,14 +110,32 @@ int of_parse_partitions(struct cdev *cdev, struct
>> device_node *node)
>> return 0;
>> }
>>
>> +/**
>> + * of_partition_ensure_probed - ensure a parition is probed
>> + * @np: pointer to a partition or to a partitionable device
>> + * Unfortunately, there is no completely reliable way
>> + * to differentiate partitions from devices prior to
>> + * probing, because partitions may also have compatibles.
>> + * We only handle nvmem-cells, so anything besides that
>> + * is assumed to be a device that should be probed directly.
>> + *
>> + * Returns zero on success or a negative error code otherwise
>> + */
>> int of_partition_ensure_probed(struct device_node *np)
>> {
>> -np = of_get_parent(np);
>> +struct device_node *parent = of_get_parent(np);
>>
>> -if (of_device_is_compatible(np, "fixed-partitions"))
>> -np = of_get_parent(np);
>> +if (parent && of_device_is_compatible(parent, "fixed-partitions"))
>
> When is the parent not present? This should only be the case when 'np'
> points to the root_node. So in case of !parent I would return -EINVAL
> early.
will do.
>
>> +return of_device_ensure_probed(of_get_parent(np));
>^
> parent?
Yes, You're right.
> Not related to this patch but the logic would become easier if would
> have devices for each mtd-part, like the kernel does. In such case we
> could avoid these special handlings and just use
> of_device_ensure_probed() at least for the mtd-parts, nvmem-cells still
> need a special handling.
How you mean? of_partition_ensure_probed can be called before there can
be any devices at all.
>
> Regards,
> Marco
>
>> -return np ? of_device_ensure_probed(np) : -EINVAL;
>> +if (of_get_compatible_child(np, "fixed-partitions"))
>> +return of_device_ensure_probed(np);
I think this can be dropped, so the case falls through to the final
of_device_ensure_probed.
>> +
>> +if (!of_property_present(np, "compatible") ||
>> +of_device_is_compatible(np, "nvmem-cells"))
>> +return of_device_ensure_probed(parent);
>> +
>> +return of_device_ensure_probed(np);
>> }
>> EXPORT_SYMBOL_GPL(of_partition_ensure_probed);
Thanks,
Ahmad
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index ab737629325a..78b8a31331db 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -484,7 +484,7 @@ int of_device_ensure_probed(struct device_node *np)
>> {
>> struct device *dev;
>>
>> -if (!deep_probe_is_supported())
>> +if (!np || !deep_probe_is_supported())
>> return 0;
>>
>> dev = of_device_create_on_demand(np);
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 18/18] state: allow lookup of barebox state partition by Type GUID
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> The backend device tree property so far always pointed at a partition.
> Let's allow pointing it at GPT storage devices directly and lookup
> the correct barebox state partition by the well-known type GUID:
>
> 4778ed65-bf42-45fa-9c5b-287a1dc4aab1
we should add an example within the Documetation/ too.
> Signed-off-by: Ahmad Fatoum
> ---
> common/state/state.c | 22 ++
> include/driver.h | 17 +
> include/state.h | 4
> 3 files changed, 43 insertions(+)
>
> diff --git a/common/state/state.c b/common/state/state.c
> index 88e246198fb8..8f56c60b0e82 100644
> --- a/common/state/state.c
> +++ b/common/state/state.c
> @@ -21,8 +21,10 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -595,6 +597,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t
> *offset, size_t *size)
> }
> #endif
>
> +static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
> +
> /*
> * state_new_from_node - create a new state instance from a device_node
> *
> @@ -641,6 +645,24 @@ struct state *state_new_from_node(struct device_node
> *node, bool readonly)
> goto out_release_state;
> }
>
> + /* Is the backend referencing an on-disk partitonable block device? */
> + if (cdev_is_block_disk(cdev)) {
> + struct cdev *partcdev = NULL;
> +
> + if (cdev_is_gpt_partitioned(cdev))
> + partcdev = cdev_find_child_by_typeuuid(cdev,
> _state_partition_guid);
> +
> + if (!partcdev) {
> + ret = -EINVAL;
> + goto out_release_state;
> + }
> +
> + pr_debug("%s: backend GPT partition looked up via
> PartitionTypeGUID\n",
> + node->full_name);
> +
> + cdev = partcdev;
> + }
What about having the above logic within a seperate function and the
above code would be:
if (cdev_is_block_disk(cdev))
cdev = get_cdev_by_typeuuid(cdev, _state_partition_guid)
if (!cdev) {
ret = -EINVAL;
goto out_release_state;
}
This way we would have everything in place to re-use the same logic for
the barebox-environmnet too. What do you think?
Regards,
Marco
> +
> state->backend_path = cdev_to_devpath(cdev, , );
>
> pr_debug("%s: backend resolved to %s\n", node->full_name,
> state->backend_path);
> diff --git a/include/driver.h b/include/driver.h
> index 6407f7d6ba36..579b03fbac34 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -585,6 +585,23 @@ extern struct list_head cdev_list;
> #define for_each_cdev(cdev) \
> list_for_each_entry(cdev, _list, list)
>
> +#define for_each_cdev_partition(partcdev, cdev) \
> + list_for_each_entry((partcdev), &(cdev)->partitions, partition_entry)
> +
> +
> +static inline struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev,
> +guid_t *typeuuid)
> +{
> + struct cdev *partcdev;
> +
> + for_each_cdev_partition(partcdev, cdev) {
> + if (guid_equal(>typeuuid, typeuuid))
> + return partcdev;
> + }
> +
> + return NULL;
> +}
> +
> #define DEVFS_PARTITION_FIXED(1U << 0)
> #define DEVFS_PARTITION_READONLY (1U << 1)
> #define DEVFS_IS_CHARACTER_DEV (1U << 3)
> diff --git a/include/state.h b/include/state.h
> index bffcd5a9007f..3daf82c0735f 100644
> --- a/include/state.h
> +++ b/include/state.h
> @@ -62,4 +62,8 @@ static inline int state_read_mac(struct state *state, const
> char *name, u8 *buf)
>
> #endif /* #if IS_ENABLED(CONFIG_STATE) / #else */
>
> +#define BAREBOX_STATE_PARTITION_GUID \
> + GUID_INIT(0x4778ed65, 0xbf42, 0x45fa, 0x9c, 0x5b, \
> + 0x28, 0x7a, 0x1d, 0xc4, 0xaa, 0xb1)
> +
> #endif /* __STATE_H */
> --
> 2.39.2
>
>
>
Re: [PATCH 16/18] cdev: use cdev::dos_partition_type only if cdev_is_mbr_partitioned
On 23-05-31, Ahmad Fatoum wrote:
> dos_partition_type == 0 can mean that either a partition is not
> a MBR partition or that it indeed has a partition type of 0x00.
>
> In preparation for using that field in a union, explicitly check if we
> have a MBR partition.
>
> Signed-off-by: Ahmad Fatoum
> ---
> common/blspec.c | 2 +-
> fs/fs.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/blspec.c b/common/blspec.c
> index e95a8dba8d76..8c7970da8915 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -729,7 +729,7 @@ int blspec_scan_device(struct bootentries *bootentries,
> struct device *dev)
>* partition with the MBR type id of 0xEA already exists it
>* should be used as $BOOT
>*/
> - if (cdev->dos_partition_type == 0xea) {
> + if (cdev_is_mbr_partitioned(cdev->master) &&
> cdev->dos_partition_type == 0xea) {
Since you already various helpers to drop priv direct access, what
about:
if (cdev_dos_partition_type(cdev) == 0xea)
Within the helper you can check for the cdev_is_mbr_partitioned().
?
Regards,
Marco
> ret = blspec_scan_cdev(bootentries, cdev);
> if (ret == 0)
> ret = -ENOENT;
> diff --git a/fs/fs.c b/fs/fs.c
> index 2d2d327c5fbc..9a92e6e251e5 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -108,7 +108,7 @@ void cdev_print(const struct cdev *cdev)
>
> if (cdev->filetype)
> nbytes += printf("Filetype: %s\t",
> file_type_to_string(cdev->filetype));
> - if (cdev->dos_partition_type)
> + if (cdev_is_mbr_partitioned(cdev->master))
> nbytes += printf("DOS parttype: 0x%02x\t",
> cdev->dos_partition_type);
> if (*cdev->partuuid || *cdev->diskuuid)
> nbytes += printf("%sUUID: %s", cdev_is_partition(cdev) ? "PART"
> : "DISK",
> --
> 2.39.2
>
>
>
Re: [PATCH v2] Porting barebox to a new SoC
On 31.05.23 19:55, Ahmad Fatoum wrote: > Hi Lior, > > On 31.05.23 18:13, Lior Weintraub wrote: >> Hi Ahmad, >> >> Using end of SRAM as PBL stack is currently not working because we need >> 40bit address (0xC0_0020_). >> This needs a fix to ENTRY_FUNCTION_WITHSTACK macro. > > Ah, right. I just sent an (untested) patch. Would be cool if you could > try it out. > >> I tried just to change const u32 __stack_top = (stack_top); to const u64 >> __stack_top = (stack_top); but there were linking errors caused by: >> ASSERT(. - __pbl_board_stack_top <= 8, "Only One PBL per Image allowed") > > The easy way would have been using a __attribute__((naked)) function, but > those are only supported for arm32, not arm64. The alternative is thus > writing the entry point in assembly and to make board authors life easier > the linker script ensures that the stack setup entry point is invoked prior > to the board entry. > >> Regarding the arm timer: >> Adding the timer entry to DT solved the issue. >> >> Regarding the UART: >> Indeed CONFIG_SERIAL_AMBA_PL011 was set on spider_defconfig (also verified >> it generated the correct entry on .config). >> I also noticed that if I remove the putc_ll implementation there is no Tx at >> all from Barebox. > > I've led you astray. Indeed: > of_platform_bus_create() - skipping /soc, no compatible prop > > points at the issue. > > Try adding into /soc > > compatible = "simple,bus"; "simple-bus". If you need an example, check out aforementioned jh7100.dtsi > ranges; > dma-ranges; > > This matches /soc with the simple bus driver which just instantiates devices > for the > contained children. Those in turn should be matched by the drivers. > > The ranges stuff just tells that memory and SoC peripherals exist in the same > address space. > > When it probes you may need to describe the clock in the DT. Eventually, you > will > want to have a clock driver for your hardware (good news: barebox and Linux > have > basically the same API, so you only need to write it once). But for now, you > can > fake it using fixed-clock in the DT. Take a look at: > > snps,dw-apb-uart in arch/riscv/dts/jh7100.dtsi and compare with > dts/src/arm/imx28.dtsi > to see how to map that to PL011. > >> Could it be a hint? > > DEBUG_LL bridges the gap until a driver registers a console that's enabled. > If this > never happens, you are left with a non-interactive shell. > > Cheers, > Ahmad > >> >> Thanks, >> Lior. >> >>> -Original Message- >>> From: Ahmad Fatoum >>> Sent: Wednesday, May 31, 2023 11:40 AM >>> To: Lior Weintraub ; Ahmad Fatoum ; >>> barebox@lists.infradead.org >>> Subject: Re: [PATCH v2] Porting barebox to a new SoC >>> >>> CAUTION: External Sender >>> >>> Hi Lior, >>> >>> On 31.05.23 10:05, Lior Weintraub wrote: Hi Ahmad, Thanks again for your prompt reply and accurate tips! Took the following changes: 1. Increasing the DRAM size to 128MB (let barebox start from 0). 2. Set PBL stack to offset 2MB from DRAM >>> >>> Just use end of SRAM, so you are completely independent of DRAM >>> until you call barebox_arm_entry. >>> 3. Fix the device tree "memory" entry to have 128MB. >>> >>> (y) >>> 4. Load barebox-spider-mk1-evk.img to SRAM and run from there. Now I can see the following logs: uncompress.c: memory at 0x, size 0x0800 uncompress.c: uncompressing barebox binary at 0x00c021c0 >>> (size 0x0001e3a4) to 0x07e0 (uncompressed size: 0x000390d0) uncompress.c: jumping to uncompressed image at 0x07e0 start.c: memory at 0x, size 0x0800 start.c: found DTB in boarddata, copying to 0x07dffcc0 start.c: initializing malloc pool at 0x079ffcc0 (size 0x0040) start.c: starting barebox... initcall-> command_slice_init+0x0/0x3c initcall-> globalvar_init+0x0/0x80 register_device: global register_device: nv initcall-> platform_init+0x0/0x1c register_device: platform initcall-> amba_bus_init+0x0/0x1c register_device: amba initcall-> spi_bus_init+0x0/0x1c register_device: spi initcall-> gpio_desc_alloc+0x0/0x24 initcall-> fs_bus_init+0x0/0x1c register_device: fs initcall-> aarch64_init_vectors+0x0/0x50 initcall-> gpio_gate_clock_driver_register+0x0/0x1c register_driver: gpio-gate-clock initcall-> of_arm_init+0x0/0x5c start.c: barebox_arm_boot_dtb: using barebox_boarddata using boarddata provided DTB adding DT alias:serial0: stem=serial id=0 node=/soc/serial@d000307000 register_device: machine of_platform_bus_create() - skipping /chosen, no compatible prop of_platform_bus_create() - skipping /aliases, no compatible prop of_platform_bus_create() - skipping /cpus, no compatible prop of_platform_bus_create() - skipping /memory@0, no compatible prop of_platform_bus_create() - skipping /soc, no compatible prop
Re: [PATCH v2] Porting barebox to a new SoC
Hi Lior, On 31.05.23 18:13, Lior Weintraub wrote: > Hi Ahmad, > > Using end of SRAM as PBL stack is currently not working because we need 40bit > address (0xC0_0020_). > This needs a fix to ENTRY_FUNCTION_WITHSTACK macro. Ah, right. I just sent an (untested) patch. Would be cool if you could try it out. > I tried just to change const u32 __stack_top = (stack_top); to const u64 > __stack_top = (stack_top); but there were linking errors caused by: > ASSERT(. - __pbl_board_stack_top <= 8, "Only One PBL per Image allowed") The easy way would have been using a __attribute__((naked)) function, but those are only supported for arm32, not arm64. The alternative is thus writing the entry point in assembly and to make board authors life easier the linker script ensures that the stack setup entry point is invoked prior to the board entry. > Regarding the arm timer: > Adding the timer entry to DT solved the issue. > > Regarding the UART: > Indeed CONFIG_SERIAL_AMBA_PL011 was set on spider_defconfig (also verified it > generated the correct entry on .config). > I also noticed that if I remove the putc_ll implementation there is no Tx at > all from Barebox. I've led you astray. Indeed: >>> of_platform_bus_create() - skipping /soc, no compatible prop points at the issue. Try adding into /soc compatible = "simple,bus"; ranges; dma-ranges; This matches /soc with the simple bus driver which just instantiates devices for the contained children. Those in turn should be matched by the drivers. The ranges stuff just tells that memory and SoC peripherals exist in the same address space. When it probes you may need to describe the clock in the DT. Eventually, you will want to have a clock driver for your hardware (good news: barebox and Linux have basically the same API, so you only need to write it once). But for now, you can fake it using fixed-clock in the DT. Take a look at: snps,dw-apb-uart in arch/riscv/dts/jh7100.dtsi and compare with dts/src/arm/imx28.dtsi to see how to map that to PL011. > Could it be a hint? DEBUG_LL bridges the gap until a driver registers a console that's enabled. If this never happens, you are left with a non-interactive shell. Cheers, Ahmad > > Thanks, > Lior. > >> -Original Message- >> From: Ahmad Fatoum >> Sent: Wednesday, May 31, 2023 11:40 AM >> To: Lior Weintraub ; Ahmad Fatoum ; >> barebox@lists.infradead.org >> Subject: Re: [PATCH v2] Porting barebox to a new SoC >> >> CAUTION: External Sender >> >> Hi Lior, >> >> On 31.05.23 10:05, Lior Weintraub wrote: >>> Hi Ahmad, >>> >>> Thanks again for your prompt reply and accurate tips! >>> Took the following changes: >>> 1. Increasing the DRAM size to 128MB (let barebox start from 0). >>> 2. Set PBL stack to offset 2MB from DRAM >> >> Just use end of SRAM, so you are completely independent of DRAM >> until you call barebox_arm_entry. >> >>> 3. Fix the device tree "memory" entry to have 128MB. >> >> (y) >> >>> 4. Load barebox-spider-mk1-evk.img to SRAM and run from there. >>> >>> Now I can see the following logs: >>> uncompress.c: memory at 0x, size 0x0800 >>> uncompress.c: uncompressing barebox binary at 0x00c021c0 >> (size 0x0001e3a4) to 0x07e0 (uncompressed size: 0x000390d0) >>> uncompress.c: jumping to uncompressed image at 0x07e0 >>> start.c: memory at 0x, size 0x0800 >>> start.c: found DTB in boarddata, copying to 0x07dffcc0 >>> start.c: initializing malloc pool at 0x079ffcc0 (size 0x0040) >>> start.c: starting barebox... >>> initcall-> command_slice_init+0x0/0x3c >>> initcall-> globalvar_init+0x0/0x80 >>> register_device: global >>> register_device: nv >>> initcall-> platform_init+0x0/0x1c >>> register_device: platform >>> initcall-> amba_bus_init+0x0/0x1c >>> register_device: amba >>> initcall-> spi_bus_init+0x0/0x1c >>> register_device: spi >>> initcall-> gpio_desc_alloc+0x0/0x24 >>> initcall-> fs_bus_init+0x0/0x1c >>> register_device: fs >>> initcall-> aarch64_init_vectors+0x0/0x50 >>> initcall-> gpio_gate_clock_driver_register+0x0/0x1c >>> register_driver: gpio-gate-clock >>> initcall-> of_arm_init+0x0/0x5c >>> start.c: barebox_arm_boot_dtb: using barebox_boarddata >>> using boarddata provided DTB >>> adding DT alias:serial0: stem=serial id=0 node=/soc/serial@d000307000 >>> register_device: machine >>> of_platform_bus_create() - skipping /chosen, no compatible prop >>> of_platform_bus_create() - skipping /aliases, no compatible prop >>> of_platform_bus_create() - skipping /cpus, no compatible prop >>> of_platform_bus_create() - skipping /memory@0, no compatible prop >>> of_platform_bus_create() - skipping /soc, no compatible prop >>> initcall-> register_autoboot_vars+0x0/0x70 >>> initcall-> arm_arch_timer_driver_register+0x0/0x1c >>> register_driver: arm-architected-timer >>> initcall-> of_timer_init+0x0/0x20 >>> initcall-> init_fs+0x0/0x3c >>> initcall-> pl011_driver_register+0x0/0x1c >>> register_driver: uart-pl011 >>> initcall->
Re: [PATCH 15/18] state: factor device path lookup into helper function
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> The #ifdef __BAREBOX__ is meant for easier synchronization with
> dt-utils. We'll keep that intact, but move it out of the function to not
> break reading flow. After sync, dt-utils would now need to implement
>
> of_cdev_find
> cdev_to_devpath
>
> Signed-off-by: Ahmad Fatoum
> ---
> common/state/state.c | 30 +++---
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/common/state/state.c b/common/state/state.c
> index 11cc86ff73be..88e246198fb8 100644
> --- a/common/state/state.c
> +++ b/common/state/state.c
> @@ -581,6 +581,20 @@ void state_release(struct state *state)
> free(state);
> }
>
> +#ifdef __BAREBOX__
> +static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
> +{
> + /*
> + * We only accept partitions exactly mapping the barebox-state,
> + * but dt-utils may need to set non-zero values here
> + */
> + *offset = 0;
> + *size = 0;
> +
> + return basprintf("/dev/%s", cdev->name);
> +}
> +#endif
We could get rid of the #ifdef if we move this function to some barebox
internal code not shared with dt-utils.
Regards,
Marco
> +
> /*
> * state_new_from_node - create a new state instance from a device_node
> *
> @@ -597,8 +611,9 @@ struct state *state_new_from_node(struct device_node
> *node, bool readonly)
> const char *alias;
> uint32_t stridesize;
> struct device_node *partition_node;
> - off_t offset = 0;
> - size_t size = 0;
> + struct cdev *cdev;
> + off_t offset;
> + size_t size;
>
> alias = of_alias_get(node);
> if (!alias) {
> @@ -617,11 +632,8 @@ struct state *state_new_from_node(struct device_node
> *node, bool readonly)
> goto out_release_state;
> }
>
> -#ifdef __BAREBOX__
> - ret = of_find_path_by_node(partition_node, >backend_path, 0);
> -#else
> - ret = of_get_devicepath(partition_node, >backend_path, ,
> );
> -#endif
> + cdev = of_cdev_find(partition_node);
> + ret = PTR_ERR_OR_ZERO(cdev);
> if (ret) {
> if (ret != -EPROBE_DEFER)
> dev_err(>dev, "state failed to parse path to
> backend: %s\n",
> @@ -629,6 +641,10 @@ struct state *state_new_from_node(struct device_node
> *node, bool readonly)
> goto out_release_state;
> }
>
> + state->backend_path = cdev_to_devpath(cdev, , );
> +
> + pr_debug("%s: backend resolved to %s\n", node->full_name,
> state->backend_path);
> +
> state->backend_reproducible_name =
> of_get_reproducible_name(partition_node);
>
> ret = of_property_read_string(node, "backend-type", _type);
> --
> 2.39.2
>
>
>
[PATCH RFT] ARM64: cpu: support 64-bit stack top in ENTRY_FUNCTION_WITHSTACK
ENTRY_FUNCTION_WITHSTACK was written with the naive assumption that
there will always be some memory in the first 32-bit of the address
space to be used as early stack. There are SoCs out there though with
sole on-chip SRAM > 4G. Accommodate this by accepting full 64-bit stack
pointers in ENTRY_FUNCTION_WITHSTACK.
Signed-off-by: Ahmad Fatoum
---
arch/arm/cpu/head_64.S | 2 +-
arch/arm/include/asm/barebox-arm.h | 2 +-
arch/arm/lib/pbl.lds.S | 7 ---
include/asm-generic/pointer.h | 2 ++
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/head_64.S b/arch/arm/cpu/head_64.S
index 398c4d3471e0..546efc263a06 100644
--- a/arch/arm/cpu/head_64.S
+++ b/arch/arm/cpu/head_64.S
@@ -11,7 +11,7 @@
ENTRY(__barebox_arm64_head)
nop
adr x9, __pbl_board_stack_top
- ldr w9, [x9]
+ ldr x9, [x9]
cbz x9, 1f
mov sp, x9
1:
diff --git a/arch/arm/include/asm/barebox-arm.h
b/arch/arm/include/asm/barebox-arm.h
index eb31ca278821..aceb7fdf74f8 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -158,7 +158,7 @@ void __barebox_arm64_head(ulong x0, ulong x1, ulong x2);
(ulong r0, ulong r1, ulong r2) \
{ \
static __section(.pbl_board_stack_top_##name) \
- const u32 __stack_top = (stack_top);\
+ const ulong __stack_top = (stack_top); \
__keep_symbolref(__barebox_arm64_head); \
__keep_symbolref(__stack_top); \
__##name(r0, r1, r2); \
diff --git a/arch/arm/lib/pbl.lds.S b/arch/arm/lib/pbl.lds.S
index 114ec7bc8195..2b4b1d6a9513 100644
--- a/arch/arm/lib/pbl.lds.S
+++ b/arch/arm/lib/pbl.lds.S
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_PBL_RELOCATABLE
#define BASE 0x0
@@ -44,14 +45,14 @@ SECTIONS
. = ALIGN(4);
.rodata : { *(.rodata*) }
- . = ALIGN(4);
+ . = ALIGN(ASM_SZPTR);
__pbl_board_stack_top = .;
.rodata.pbl_board_stack_top : {
*(.pbl_board_stack_top_*)
/* Dummy for when BootROM sets up usable stack */
- LONG(0x);
+ ASM_LD_PTR(0x)
}
- ASSERT(. - __pbl_board_stack_top <= 8, "Only One PBL per Image allowed")
+ ASSERT(. - __pbl_board_stack_top <= 2 * ASM_SZPTR, "Only One PBL per
Image allowed")
.barebox_imd : { BAREBOX_IMD }
diff --git a/include/asm-generic/pointer.h b/include/asm-generic/pointer.h
index 8b9600b02939..89817ce59ebc 100644
--- a/include/asm-generic/pointer.h
+++ b/include/asm-generic/pointer.h
@@ -8,6 +8,7 @@
#define ASM_PTR.quad
#define ASM_SZPTR 8
#define ASM_LGPTR 3
+#define ASM_LD_PTR(x) QUAD(x)
#else
#define ASM_PTR".quad"
#define ASM_SZPTR "8"
@@ -18,6 +19,7 @@
#define ASM_PTR.word
#define ASM_SZPTR 4
#define ASM_LGPTR 2
+#define ASM_LD_PTR(x) LONG(x)
#else
#define ASM_PTR".word"
#define ASM_SZPTR "4"
--
2.39.2
Re: [PATCH 14/18] of: export new of_cdev_find helper
On 23-05-31, Ahmad Fatoum wrote:
> __of_find_path goes throught the hassle of determining the cdev, only to
> discard it again and return either zero or an error code.
>
> Follow up commits will need to get the cdev corresponding to a path in
> the DT. So let's make that easier by exporting a suitable helper function.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> drivers/of/of_path.c | 59 ++--
> include/of.h | 1 +
> 2 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
> index 059690e9b8e8..4d9f7b6005af 100644
> --- a/drivers/of/of_path.c
> +++ b/drivers/of/of_path.c
> @@ -27,21 +27,17 @@ struct device *of_find_device_by_node_path(const char
> *path)
> }
>
> /**
> - * __of_find_path
> + * __of_cdev_find
> *
> * @node: The node to find the cdev for, can be the device or a
> *partition in the device
> * @part: Optionally, a description of a partition of @node. See
> of_find_path
> - * @outpath: if this function returns 0 outpath will contain the path
> belonging
> - * to the input path description. Must be freed with free().
> - * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
> *
> */
> -static int __of_find_path(struct device_node *node, const char *part, char
> **outpath, unsigned flags)
> +static struct cdev *__of_cdev_find(struct device_node *node, const char
> *part)
> {
> struct device *dev;
> struct cdev *cdev;
> - bool add_bb = false;
>
> of_partition_ensure_probed(node);
>
> @@ -56,24 +52,17 @@ static int __of_find_path(struct device_node *node, const
> char *part, char **out
>
> /* when partuuid is specified short-circuit the search
> for the cdev */
> ret = of_property_read_string(node, "partuuid", );
> - if (!ret) {
> - cdev = cdev_by_partuuid(uuid);
> - if (!cdev)
> - return -ENODEV;
> -
> - *outpath = basprintf("/dev/%s", cdev->name);
> -
> - return 0;
> - }
> + if (!ret)
> + return cdev_by_partuuid(uuid) ?:
> ERR_PTR(-ENODEV);
> }
>
> dev = of_find_device_by_node_path(devnode->full_name);
> if (!dev)
> - return -ENODEV;
> + return ERR_PTR(-ENODEV);
> }
>
> if (dev->bus && !dev->driver)
> - return -EPROBE_DEFER;
> + return ERR_PTR(-EPROBE_DEFER);
>
> device_detect(dev);
>
> @@ -82,8 +71,40 @@ static int __of_find_path(struct device_node *node, const
> char *part, char **out
> else
> cdev = cdev_by_device_node(node);
>
> - if (!cdev)
> - return -ENOENT;
> + return cdev ?: ERR_PTR(-ENOENT);
> +}
> +
> +/**
> + * of_cdev_find
> + *
> + * @node: The node to find the cdev for, can be the device or a
> + *partition in the device
> + *
> + */
> +struct cdev *of_cdev_find(struct device_node *node)
> +{
> + return __of_cdev_find(node, NULL);
> +}
> +
> +/**
> + * __of_find_path
> + *
> + * @node: The node to find the cdev for, can be the device or a
> + *partition in the device
> + * @part: Optionally, a description of a partition of @node. See
> of_find_path
> + * @outpath: if this function returns 0 outpath will contain the path
> belonging
> + * to the input path description. Must be freed with free().
> + * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
> + *
> + */
> +static int __of_find_path(struct device_node *node, const char *part, char
> **outpath, unsigned flags)
> +{
> + bool add_bb = false;
> + struct cdev *cdev;
> +
> + cdev = __of_cdev_find(node, part);
> + if (IS_ERR(cdev))
> + return PTR_ERR(cdev);
>
> if ((flags & OF_FIND_PATH_FLAGS_BB) && cdev->mtd &&
> mtd_can_have_bb(cdev->mtd))
> diff --git a/include/of.h b/include/of.h
> index c716f9283316..2b75ce63e185 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -331,6 +331,7 @@ int of_add_memory_bank(struct device_node *node, bool
> dump, int r,
> struct device *of_find_device_by_node_path(const char *path);
> #define OF_FIND_PATH_FLAGS_BB 1 /* return .bb device if
> available */
> int of_find_path(struct device_node *node, const char *propname, char
> **outpath, unsigned flags);
> +struct cdev *of_cdev_find(struct device_node *node);
> int of_find_path_by_node(struct device_node *node, char **outpath, unsigned
> flags);
> struct device_node *of_find_node_by_devpath(struct device_node *root, const
> char *path);
> int of_register_fixup(int (*fixup)(struct device_node *, void *), void
> *context);
> --
> 2.39.2
>
>
>
Re: [PATCH 13/18] block: add cdev_is_block_(device,partition,disk) helpers
On 23-05-31, Ahmad Fatoum wrote:
> We look too much into struct cdev's guts. Let's add helpers to make
> operating on block device cdevs more concise.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> include/block.h | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/include/block.h b/include/block.h
> index 4dd2aa1f80ef..da258f509b41 100644
> --- a/include/block.h
> +++ b/include/block.h
> @@ -58,4 +58,19 @@ static inline struct block_device
> *cdev_get_block_device(const struct cdev *cdev
> }
> #endif
>
> +static inline bool cdev_is_block_device(const struct cdev *cdev)
> +{
> + return cdev_get_block_device(cdev) != NULL;
> +}
> +
> +static inline bool cdev_is_block_partition(const struct cdev *cdev)
> +{
> + return cdev_is_block_device(cdev) && cdev_is_partition(cdev);
> +}
> +
> +static inline bool cdev_is_block_disk(const struct cdev *cdev)
> +{
> + return cdev_is_block_device(cdev) && !cdev_is_partition(cdev);
> +}
> +
> #endif /* __BLOCK_H */
> --
> 2.39.2
>
>
>
Re: [PATCH 12/18] common: partitions: record whether disk is GPT or MBR partitioned
On 23-05-31, Ahmad Fatoum wrote:
> Currently, the only way to differentiate between a GPT disk and a MBR
> one is to check whether the cdev's device has a guid (=> GPT) or a
> nt_signature (=> MBR) device parameter. We already have a flag parameter
> though, so let's record this info there for easy retrieval.
>
> We intentionally don't use the struct cdev::filetype member, because
> we don't want to change behavior of file_detect_type().
>
> Signed-off-by: Ahmad Fatoum
> ---
> common/partitions/dos.c | 2 ++
> common/partitions/efi.c | 2 ++
> fs/fs.c | 4
> include/driver.h| 20 +---
> 4 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/common/partitions/dos.c b/common/partitions/dos.c
> index ad60c0b27b46..7472824b00b9 100644
> --- a/common/partitions/dos.c
> +++ b/common/partitions/dos.c
> @@ -185,6 +185,8 @@ static void dos_partition(void *buf, struct block_device
> *blk,
> if (signature)
> sprintf(blk->cdev.diskuuid, "%08x", signature);
>
> + blk->cdev.flags |= DEVFS_IS_MBR_PARTITIONED;
> +
> table = (struct partition_entry *)[446];
>
> for (i = 0; i < 4; i++) {
> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
> index 780a8695e8a8..df63b82afe24 100644
> --- a/common/partitions/efi.c
> +++ b/common/partitions/efi.c
> @@ -449,6 +449,8 @@ static void efi_partition(void *buf, struct block_device
> *blk,
> snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl",
> >disk_guid);
> dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
>
> + blk->cdev.flags |= DEVFS_IS_GPT_PARTITIONED;
> +
> nb_part = le32_to_cpu(gpt->num_partition_entries);
>
> if (nb_part > MAX_PARTITION) {
> diff --git a/fs/fs.c b/fs/fs.c
> index 9d8aab268ca4..2d2d327c5fbc 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -94,6 +94,10 @@ void cdev_print(const struct cdev *cdev)
> printf(" table-partition");
> if (cdev->flags & DEVFS_IS_MCI_MAIN_PART_DEV)
> printf(" mci-main-partition");
> + if (cdev->flags & DEVFS_IS_GPT_PARTITIONED)
> + printf(" gpt-partitioned");
> + if (cdev->flags & DEVFS_IS_MBR_PARTITIONED)
> + printf(" mbr-partitioned");
> if (cdev->mtd)
> printf(" mtd");
> printf(" )");
> diff --git a/include/driver.h b/include/driver.h
> index 118d2adb6750..5f2eae65466f 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -584,9 +584,23 @@ extern struct list_head cdev_list;
> #define DEVFS_PARTITION_FIXED(1U << 0)
> #define DEVFS_PARTITION_READONLY (1U << 1)
> #define DEVFS_IS_CHARACTER_DEV (1U << 3)
> -#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 4)
> -#define DEVFS_PARTITION_FROM_OF (1U << 5)
> -#define DEVFS_PARTITION_FROM_TABLE (1U << 6)
> +#define DEVFS_PARTITION_FROM_OF (1U << 4)
> +#define DEVFS_PARTITION_FROM_TABLE (1U << 5)
> +#define DEVFS_IS_GPT_PARTITIONED (1U << 6)
> +#define DEVFS_IS_MBR_PARTITIONED (1U << 7)
> +#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 8)
Why do you reorder the bits here again?
> +
> +static inline bool cdev_is_gpt_partitioned(const struct cdev *master)
> +{
> + return master && (master->flags & DEVFS_IS_GPT_PARTITIONED)
> + == DEVFS_IS_GPT_PARTITIONED;
Why not just: 'return !!(master->flags & DEVFS_IS_GPT_PARTITIONED)' ?
Regards,
Marco
> +}
> +
> +static inline bool cdev_is_mbr_partitioned(const struct cdev *master)
> +{
> + return master && (master->flags & DEVFS_IS_MBR_PARTITIONED)
> + == DEVFS_IS_MBR_PARTITIONED;
> +}
>
> struct cdev *devfs_add_partition(const char *devname, loff_t offset,
> loff_t size, unsigned int flags, const char *name);
> --
> 2.39.2
>
>
>
Re: [PATCH 11/18] block: parse partition table on block device registration
On 23-05-31, Ahmad Fatoum wrote:
> Every instance where we register a block device, it's followed by an
> attempt to parse the partition table, most often with a warning when
> it fails. Thus let's move partition table parsing into
> blockdevice_register.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> arch/sandbox/board/hostfile.c | 4
> common/block.c| 6 ++
> drivers/ata/disk_ata_drive.c | 5 -
> drivers/block/efi-block-io.c | 9 +
> drivers/block/virtio_blk.c| 8 +---
> drivers/mci/mci-core.c| 6 --
> drivers/nvme/host/core.c | 5 -
> drivers/usb/storage/usb.c | 5 -
> 8 files changed, 8 insertions(+), 40 deletions(-)
>
> diff --git a/arch/sandbox/board/hostfile.c b/arch/sandbox/board/hostfile.c
> index d0f400787d7a..a1ab06b87770 100644
> --- a/arch/sandbox/board/hostfile.c
> +++ b/arch/sandbox/board/hostfile.c
> @@ -166,10 +166,6 @@ static int hf_probe(struct device *dev)
> if (err)
> return err;
>
> - err = parse_partition_table(>blk);
> - if (err)
> - dev_warn(dev, "No partition table found\n");
> -
> dev_info(dev, "registered as block device\n");
> } else {
> cdev->name = np->name;
> diff --git a/common/block.c b/common/block.c
> index c39269d3a692..98adcfdf3dab 100644
> --- a/common/block.c
> +++ b/common/block.c
> @@ -6,6 +6,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -408,6 +409,11 @@ int blockdevice_register(struct block_device *blk)
>
> cdev_create_default_automount(>cdev);
>
> + /* Lack of partition table is unusual, but not a failure */
> + ret = parse_partition_table(blk);
> + if (ret)
> + dev_warn(blk->dev, "No partition table found\n");
> +
> return 0;
> }
>
> diff --git a/drivers/ata/disk_ata_drive.c b/drivers/ata/disk_ata_drive.c
> index c1c736a0a88a..2d97710b827a 100644
> --- a/drivers/ata/disk_ata_drive.c
> +++ b/drivers/ata/disk_ata_drive.c
> @@ -254,11 +254,6 @@ static int ata_port_init(struct ata_port *port)
>
> dev_info(dev, "registered /dev/%s\n", port->blk.cdev.name);
>
> - /* create partitions on demand */
> - rc = parse_partition_table(>blk);
> - if (rc != 0)
> - dev_warn(dev, "No partition table found\n");
> -
> return 0;
>
> on_error:
> diff --git a/drivers/block/efi-block-io.c b/drivers/block/efi-block-io.c
> index eb4981e86298..7162106ab8ea 100644
> --- a/drivers/block/efi-block-io.c
> +++ b/drivers/block/efi-block-io.c
> @@ -12,7 +12,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -184,16 +183,10 @@ static int efi_bio_probe(struct efi_device *efidev)
>
> priv->media_id = media->media_id;
>
> - ret = blockdevice_register(>blk);
> - if (ret)
> - return ret;
> -
> if (efi_get_bootsource() == efidev)
> bootsource_set_raw_instance(instance);
>
> - parse_partition_table(>blk);
> -
> - return 0;
> + return blockdevice_register(>blk);
> }
>
> static struct efi_driver efi_bio_driver = {
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 660f3a7b6b9b..11e52d9e6457 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -105,13 +105,7 @@ static int virtio_blk_probe(struct virtio_device *vdev)
> priv->blk.num_blocks = cap;
> priv->blk.ops = _blk_ops;
>
> - ret = blockdevice_register(>blk);
> - if (ret)
> - return ret;
> -
> - parse_partition_table(>blk);
> -
> - return 0;
> + return blockdevice_register(>blk);
> }
>
> static void virtio_blk_remove(struct virtio_device *vdev)
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 6d0d6473770c..32edd5382386 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1900,12 +1900,6 @@ static int mci_register_partition(struct mci_part
> *part)
> return 0;
> }
>
> - rc = parse_partition_table(>blk);
> - if (rc != 0) {
> - /* Lack of partition table is unusual, but not a failure */
> - dev_warn(>dev, "No partition table found\n");
> - }
> -
> if (np) {
> of_parse_partitions(>blk.cdev, np);
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index bf9176ce0922..79a5f9325ef8 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1,6 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> #include
> -#include
>
> #include "nvme.h"
>
> @@ -373,10 +372,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl,
> unsigned nsid)
> goto out_free_id;
> }
>
> - ret = parse_partition_table(>blk);
> - if (ret)
> - dev_warn(ctrl->dev, "No partition table found\n");
> -
> return;
>
Re: [PATCH 10/18] cdev: have devfs_add_partition return existing identical partition, not NULL
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> Starting with commit 7f9f45b9bfef ("devfs: Do not create overlapping
> partitions"), any overlapping is disallowed. Overlapping can be useful
> though to bridge the gap between partition described in DT and via
> on-disk partition tables. Let's handle the case of identical partitions
> specially and have it neither be an error or a duplicate partition, but
> instead just return the existing partition. This existing partition will
> be given a device tree node and thus enabling schemes like:
>
> &{/state} {
> backend = <_part>;
> };
>
>{
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <2>;
> #size-cells = <2>;
>
> state_part: partition@530 {
> label = "barebox-state";
>/* will be folded with overlapping GPT partition if
> found */
> reg = <0x0 0x530 0x0 0x10>;
> };
> };
> };
>
> Signed-off-by: Ahmad Fatoum
> ---
> fs/devfs-core.c | 50 ++---
> 1 file changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> index a0732dafca42..b3a274d01ee0 100644
> --- a/fs/devfs-core.c
> +++ b/fs/devfs-core.c
> @@ -402,6 +402,12 @@ int devfs_remove(struct cdev *cdev)
> return 0;
> }
>
> +static bool region_identical(loff_t starta, loff_t lena,
> + loff_t startb, loff_t lenb)
> +{
> + return starta == startb && lena == lenb;
> +}
> +
> static bool region_overlap(loff_t starta, loff_t lena,
> loff_t startb, loff_t lenb)
> {
> @@ -412,10 +418,22 @@ static bool region_overlap(loff_t starta, loff_t lena,
> return 1;
> }
>
> -static int check_overlap(struct cdev *cdev, const char *name, loff_t offset,
> loff_t size)
> +/**
> + * check_overlap() - check overlap with existing partitions
> + * @cdev: parent cdev
> + * @name: partition name for informational purposes on conflict
> + * @offset: offset of new partition to be added
> + * @size: size of new partition to be added
> + *
> + * Return: NULL if no overlapping partition found or overlapping
> + * partition if and only if it's identical in offset and size
> + * to an existing partition. Otherwise, PTR_ERR(-EINVAL).
> + */
> +static struct cdev *check_overlap(struct cdev *cdev, const char *name,
> loff_t offset, loff_t size)
> {
> struct cdev *cpart;
> loff_t cpart_offset;
> + int ret;
>
> list_for_each_entry(cpart, >partitions, partition_entry) {
> cpart_offset = cpart->offset;
> @@ -428,20 +446,28 @@ static int check_overlap(struct cdev *cdev, const char
> *name, loff_t offset, lof
> if (cpart->mtd)
> cpart_offset = cpart->mtd->master_offset;
>
> - if (region_overlap(cpart_offset, cpart->size,
> -offset, size))
> + if (region_identical(cpart_offset, cpart->size, offset, size)) {
> + ret = 0;
> goto conflict;
> + }
The 'goto conflict' is a bit misleading here since this is no conflict
as you described within the commit message. I would rather do:
if (region_identical(cpart_offset, cpart->size, offset, size))
goto out_identical;
and replace the __pr_printk() by pr_debug(). This way you split
__pr_printk() and drop the ternary operator. The rest lgtm.
Regards,
Marco
> +
> + if (region_overlap(cpart_offset, cpart->size, offset, size)) {
> + ret = -EINVAL;
> + goto conflict;
> + }
> }
>
> - return 0;
> + return NULL;
>
> conflict:
> - pr_err("New partition %s (0x%08llx-0x%08llx) on %s "
> - "overlaps with partition %s (0x%08llx-0x%08llx), not creating
> it\n",
> - name, offset, offset + size - 1, cdev->name,
> - cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
> + __pr_printk(ret ? MSG_WARNING : MSG_DEBUG,
> + "New partition %s (0x%08llx-0x%08llx) on %s "
> + "%s with partition %s (0x%08llx-0x%08llx), not creating
> it\n",
> + name, offset, offset + size - 1, cdev->name,
> + ret ? "conflicts" : "identical",
> + cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
>
> - return -EINVAL;
> + return ret ? ERR_PTR(ret) : cpart;
> }
>
> static struct cdev *__devfs_add_partition(struct cdev *cdev,
> @@ -449,6 +475,7 @@ static struct cdev *__devfs_add_partition(struct cdev
> *cdev,
> {
> loff_t offset, size;
> static struct cdev *new;
> + struct cdev *overlap;
>
> if (cdev_by_name(partinfo->name))
> return ERR_PTR(-EEXIST);
> @@ -479,8
Re: [PATCH 09/18] cdev: record whether partition is parsed from OF
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> Later code will make it possible to define a on-disk-described partition
> in the DT as well. For this reason, we can't assumed
> DEVFS_PARTITION_FROM_TABLE to mean !DT, so let's add a dedicated flag
> for that.
>
> Signed-off-by: Ahmad Fatoum
> ---
> drivers/of/partition.c | 5 +++--
> fs/fs.c| 2 ++
> include/driver.h | 5 +++--
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/of/partition.c b/drivers/of/partition.c
> index a70e503cec9e..15943502ce17 100644
> --- a/drivers/of/partition.c
> +++ b/drivers/of/partition.c
> @@ -74,6 +74,7 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct
> device_node *node)
> }
>
> new->device_node = node;
> + new->flags |= DEVFS_PARTITION_FROM_OF;
>
> if (IS_ENABLED(CONFIG_NVMEM) && of_device_is_compatible(node,
> "nvmem-cells")) {
> struct nvmem_device *nvmem = nvmem_partition_register(new);
> @@ -162,7 +163,7 @@ int of_fixup_partitions(struct device_node *np, struct
> cdev *cdev)
> return 0;
>
> list_for_each_entry(partcdev, >partitions, partition_entry) {
> - if (partcdev->flags & DEVFS_PARTITION_FROM_TABLE)
> + if (!(partcdev->flags & DEVFS_PARTITION_FROM_OF))
Even though the code is already 'open-coded' I would suggest a macro like:
is_of_partition_cdev() or cdev_is_of_partition().
Reviewed-by: Marco Felsch
> continue;
> n_parts++;
> }
> @@ -213,7 +214,7 @@ int of_fixup_partitions(struct device_node *np, struct
> cdev *cdev)
> u8 tmp[16 * 16]; /* Up to 64-bit address + 64-bit size */
> loff_t partoffset;
>
> - if (partcdev->flags & DEVFS_PARTITION_FROM_TABLE)
> + if (!(partcdev->flags & DEVFS_PARTITION_FROM_OF))
> continue;
>
> if (partcdev->mtd)
> diff --git a/fs/fs.c b/fs/fs.c
> index 1820e48393af..9d8aab268ca4 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -88,6 +88,8 @@ void cdev_print(const struct cdev *cdev)
> printf(" fixed-partition");
> if (cdev->flags & DEVFS_PARTITION_READONLY)
> printf(" readonly-partition");
> + if (cdev->flags & DEVFS_PARTITION_FROM_OF)
> + printf(" of-partition");
> if (cdev->flags & DEVFS_PARTITION_FROM_TABLE)
> printf(" table-partition");
> if (cdev->flags & DEVFS_IS_MCI_MAIN_PART_DEV)
> diff --git a/include/driver.h b/include/driver.h
> index 42e513a15603..118d2adb6750 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -584,8 +584,9 @@ extern struct list_head cdev_list;
> #define DEVFS_PARTITION_FIXED(1U << 0)
> #define DEVFS_PARTITION_READONLY (1U << 1)
> #define DEVFS_IS_CHARACTER_DEV (1U << 3)
> -#define DEVFS_PARTITION_FROM_TABLE (1U << 4)
> -#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 5)
> +#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 4)
> +#define DEVFS_PARTITION_FROM_OF (1U << 5)
> +#define DEVFS_PARTITION_FROM_TABLE (1U << 6)
>
> struct cdev *devfs_add_partition(const char *devname, loff_t offset,
> loff_t size, unsigned int flags, const char *name);
> --
> 2.39.2
>
>
>
Re: [PATCH 08/18] cdev: use more descriptive struct cdev::diskuuid/partuuid
On 23-05-31, Ahmad Fatoum wrote:
> The UUID field has different meanings:
>
> For a master cdev:
>
> - GPT Header DiskGUID if GPT-formatted
> - MBR Header NT Disk Signature if MBR-formatted
>
> For a partition cdev:
>
> - GPT UniquePartitionGUID
> - MBR Header NT Disk Signature followed by "-${partititon_number}"
>
> Later code will add yet another UUID (Partition Type GUID), so let's
> make existing code more readable by using either diskuuid or partuuid as
> appropriate.
>
> No functional change.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> common/bootm.c | 2 +-
> common/partitions.c| 2 +-
> common/partitions/dos.c| 2 +-
> common/partitions/efi.c| 4 ++--
> drivers/misc/storage-by-uuid.c | 4 ++--
> fs/devfs-core.c| 4 ++--
> fs/fs.c| 9 +
> include/driver.h | 10 +-
> 8 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index 91a6e1688674..791d6b8fbbf1 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -734,7 +734,7 @@ int bootm_boot(struct bootm_data *bootm_data)
> if (!root_cdev)
> pr_err("no cdev found for %s, cannot
> set root= option\n",
> root_dev_name);
> - else if (!root_cdev->uuid[0])
> + else if (!root_cdev->partuuid[0])
> pr_err("%s doesn't have a PARTUUID,
> cannot set root= option\n",
> root_dev_name);
> }
> diff --git a/common/partitions.c b/common/partitions.c
> index 9cca5c4a1546..b579559672a0 100644
> --- a/common/partitions.c
> +++ b/common/partitions.c
> @@ -51,7 +51,7 @@ static int register_one_partition(struct block_device *blk,
> cdev->flags |= DEVFS_PARTITION_FROM_TABLE;
>
> cdev->dos_partition_type = part->dos_partition_type;
> - strcpy(cdev->uuid, part->partuuid);
> + strcpy(cdev->partuuid, part->partuuid);
>
> free(partition_name);
>
> diff --git a/common/partitions/dos.c b/common/partitions/dos.c
> index 566c8dd949b4..ad60c0b27b46 100644
> --- a/common/partitions/dos.c
> +++ b/common/partitions/dos.c
> @@ -183,7 +183,7 @@ static void dos_partition(void *buf, struct block_device
> *blk,
> uint32_t signature = get_unaligned_le32(buf + 0x1b8);
>
> if (signature)
> - sprintf(blk->cdev.uuid, "%08x", signature);
> + sprintf(blk->cdev.diskuuid, "%08x", signature);
>
> table = (struct partition_entry *)[446];
>
> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
> index a6c95f3969d1..780a8695e8a8 100644
> --- a/common/partitions/efi.c
> +++ b/common/partitions/efi.c
> @@ -446,8 +446,8 @@ static void efi_partition(void *buf, struct block_device
> *blk,
> return;
> }
>
> - snprintf(blk->cdev.uuid, sizeof(blk->cdev.uuid), "%pUl",
> >disk_guid);
> - dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.uuid);
> + snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl",
> >disk_guid);
> + dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
>
> nb_part = le32_to_cpu(gpt->num_partition_entries);
>
> diff --git a/drivers/misc/storage-by-uuid.c b/drivers/misc/storage-by-uuid.c
> index a938bfaaa2c4..a7a66a1421a3 100644
> --- a/drivers/misc/storage-by-uuid.c
> +++ b/drivers/misc/storage-by-uuid.c
> @@ -140,8 +140,8 @@ static void check_exist(struct sbu *sbu)
> struct cdev *cdev;
>
> for_each_cdev(cdev) {
> - if (!strcmp(cdev->uuid, sbu->uuid)) {
> - dev_dbg(sbu->dev, "Found %s %s\n", cdev->name,
> cdev->uuid);
> + if (!strcmp(cdev->diskuuid, sbu->uuid)) {
> + dev_dbg(sbu->dev, "Found %s %s\n", cdev->name,
> cdev->diskuuid);
> storage_by_uuid_add_partitions(sbu, cdev);
> }
> }
> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> index bb66993b90f9..a0732dafca42 100644
> --- a/fs/devfs-core.c
> +++ b/fs/devfs-core.c
> @@ -101,7 +101,7 @@ struct cdev *cdev_by_partuuid(const char *partuuid)
> return NULL;
>
> for_each_cdev(cdev) {
> - if (cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
> partuuid))
> + if (cdev_is_partition(cdev) && !strcasecmp(cdev->partuuid,
> partuuid))
> return cdev;
> }
> return NULL;
> @@ -115,7 +115,7 @@ struct cdev *cdev_by_diskuuid(const char *diskuuid)
> return NULL;
>
> for_each_cdev(cdev) {
> - if (!cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
> diskuuid))
> + if (!cdev_is_partition(cdev) && !strcasecmp(cdev->diskuuid,
> diskuuid))
>
Re: [PATCH 07/18] commands: stat: remove code duplication for type info
On 23-05-31, Ahmad Fatoum wrote:
> stat prints a line with partitioning/type info for cdevs, but not all
> cdevs have these, so we want to skip printing when it's empty.
> Instead of duplicating the check, just utilize printf returning the
> number of characters written.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> fs/fs.c | 20
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index 368458cc54f8..ba60766a065a 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -69,6 +69,8 @@ EXPORT_SYMBOL(mkmodestr);
>
> void cdev_print(const struct cdev *cdev)
> {
> + int nbytes;
> +
> if (cdev->dev || cdev->master || cdev->partname) {
> printf("Origin: %s", dev_name(cdev->dev) ?: "None");
> if (cdev->master)
> @@ -96,15 +98,17 @@ void cdev_print(const struct cdev *cdev)
> }
> printf("\n");
>
> - if (cdev->filetype || cdev->dos_partition_type || *cdev->uuid) {
> - if (cdev->filetype)
> - printf("Filetype: %s\t",
> file_type_to_string(cdev->filetype));
> - if (cdev->dos_partition_type)
> - printf("DOS parttype: 0x%02x\t",
> cdev->dos_partition_type);
> - if (*cdev->uuid)
> - printf("UUID: %s", cdev->uuid);
> + nbytes = 0;
> +
> + if (cdev->filetype)
> + nbytes += printf("Filetype: %s\t",
> file_type_to_string(cdev->filetype));
> + if (cdev->dos_partition_type)
> + nbytes += printf("DOS parttype: 0x%02x\t",
> cdev->dos_partition_type);
> + if (*cdev->uuid)
> + nbytes += printf("UUID: %s", cdev->uuid);
> +
> + if (nbytes)
> printf("\n");
> - }
> }
> EXPORT_SYMBOL(cdev_print);
>
> --
> 2.39.2
>
>
>
Re: [PATCH 06/18] driver: add new cdev_is_partition helper
On 23-05-31, Ahmad Fatoum wrote:
> Partitions will have cdev->master != NULL, so often code will just do
> if (cdev->master) to check if a cdev is a partition. This is suboptimal
> as it may be misinterpreted by readers as meaning that the cdev is the
> master device, while it's the other way round.
>
> Let's define cdev_is_partition instead and use it everywhere, where
> cdev->master is only checked, but not dereferenced.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> drivers/base/driver.c | 2 +-
> fs/devfs-core.c | 10 +-
> include/driver.h | 4
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index f00be99cdcbf..10d765e1a213 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -277,7 +277,7 @@ int unregister_device(struct device *old_dev)
> }
>
> list_for_each_entry_safe(cdev, ct, _dev->cdevs, devices_list) {
> - if (cdev->master) {
> + if (cdev_is_partition(cdev)) {
> dev_dbg(old_dev, "unregister part %s\n", cdev->name);
> devfs_del_partition(cdev->name);
> }
> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> index fbcf68e81597..bb66993b90f9 100644
> --- a/fs/devfs-core.c
> +++ b/fs/devfs-core.c
> @@ -38,7 +38,7 @@ int devfs_partition_complete(struct string_list *sl, char
> *instr)
> len = strlen(instr);
>
> for_each_cdev(cdev) {
> - if (cdev->master &&
> + if (cdev_is_partition(cdev) &&
> !strncmp(instr, cdev->name, len)) {
> string_list_add_asprintf(sl, "%s ", cdev->name);
> }
> @@ -101,7 +101,7 @@ struct cdev *cdev_by_partuuid(const char *partuuid)
> return NULL;
>
> for_each_cdev(cdev) {
> - if (cdev->master && !strcasecmp(cdev->uuid, partuuid))
> + if (cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
> partuuid))
> return cdev;
> }
> return NULL;
> @@ -115,7 +115,7 @@ struct cdev *cdev_by_diskuuid(const char *diskuuid)
> return NULL;
>
> for_each_cdev(cdev) {
> - if (!cdev->master && !strcasecmp(cdev->uuid, diskuuid))
> + if (!cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
> diskuuid))
> return cdev;
> }
> return NULL;
> @@ -393,7 +393,7 @@ int devfs_remove(struct cdev *cdev)
> list_for_each_entry_safe(c, tmp, >links, link_entry)
> devfs_remove(c);
>
> - if (cdev->master)
> + if (cdev_is_partition(cdev))
> list_del(>partition_entry);
>
> if (cdev->link)
> @@ -549,7 +549,7 @@ int devfs_del_partition(const char *name)
> return ret;
> }
>
> - if (!cdev->master)
> + if (!cdev_is_partition(cdev))
> return -EINVAL;
> if (cdev->flags & DEVFS_PARTITION_FIXED)
> return -EPERM;
> diff --git a/include/driver.h b/include/driver.h
> index e1ee3dc2dd7c..00b4a0e4af75 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -564,6 +564,10 @@ int cdev_discard_range(struct cdev*, loff_t count,
> loff_t offset);
> int cdev_memmap(struct cdev*, void **map, int flags);
> int cdev_truncate(struct cdev*, size_t size);
> loff_t cdev_unallocated_space(struct cdev *cdev);
> +static inline bool cdev_is_partition(const struct cdev *cdev)
> +{
> + return cdev->master != NULL;
> +}
>
> extern struct list_head cdev_list;
> #define for_each_cdev(cdev) \
> --
> 2.39.2
>
>
>
Re: [PATCH 05/18] of: of_path: always call of_partition_ensure_probed before resolving
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> of_find_path may be called on a partition, whose parent device is not
> yet probed. state code solves that by calling of_partition_ensure_probed
> before of_find_path_by_nde, but really we should be doing that for all
nit: of_find_path_by_node
> calls to of_find_path. Do so.
>
> Signed-off-by: Ahmad Fatoum
Reviewed-by: Marco Felsch
> ---
> common/state/state.c | 4
> drivers/of/of_path.c | 2 ++
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/common/state/state.c b/common/state/state.c
> index 6b4acbb32bcc..11cc86ff73be 100644
> --- a/common/state/state.c
> +++ b/common/state/state.c
> @@ -618,10 +618,6 @@ struct state *state_new_from_node(struct device_node
> *node, bool readonly)
> }
>
> #ifdef __BAREBOX__
> - ret = of_partition_ensure_probed(partition_node);
> - if (ret)
> - goto out_release_state;
> -
> ret = of_find_path_by_node(partition_node, >backend_path, 0);
> #else
> ret = of_get_devicepath(partition_node, >backend_path, ,
> );
> diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
> index 1268cf36ee5b..059690e9b8e8 100644
> --- a/drivers/of/of_path.c
> +++ b/drivers/of/of_path.c
> @@ -43,6 +43,8 @@ static int __of_find_path(struct device_node *node, const
> char *part, char **out
> struct cdev *cdev;
> bool add_bb = false;
>
> + of_partition_ensure_probed(node);
> +
> dev = of_find_device_by_node_path(node->full_name);
> if (!dev) {
> int ret;
> --
> 2.39.2
>
>
>
Re: [PATCH 04/18] of: partition: support of_partition_ensure_probed on parent device
Hi Ahmad,
On 23-05-31, Ahmad Fatoum wrote:
> barebox-state code uses of_partition_ensure_probed to resolve the
> backend property. We want to allow backend to point directly at a
> storage device instead of a partition. We can't determine whether a DT
> device is a storage device though before it's probed, so let's have
> of_partition_ensure_probed support either case.
>
> Signed-off-by: Ahmad Fatoum
> ---
> drivers/of/partition.c | 26 ++
> drivers/of/platform.c | 2 +-
> 2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/of/partition.c b/drivers/of/partition.c
> index 40c47f554ad2..a70e503cec9e 100644
> --- a/drivers/of/partition.c
> +++ b/drivers/of/partition.c
> @@ -110,14 +110,32 @@ int of_parse_partitions(struct cdev *cdev, struct
> device_node *node)
> return 0;
> }
>
> +/**
> + * of_partition_ensure_probed - ensure a parition is probed
> + * @np: pointer to a partition or to a partitionable device
> + * Unfortunately, there is no completely reliable way
> + * to differentiate partitions from devices prior to
> + * probing, because partitions may also have compatibles.
> + * We only handle nvmem-cells, so anything besides that
> + * is assumed to be a device that should be probed directly.
> + *
> + * Returns zero on success or a negative error code otherwise
> + */
> int of_partition_ensure_probed(struct device_node *np)
> {
> - np = of_get_parent(np);
> + struct device_node *parent = of_get_parent(np);
>
> - if (of_device_is_compatible(np, "fixed-partitions"))
> - np = of_get_parent(np);
> + if (parent && of_device_is_compatible(parent, "fixed-partitions"))
When is the parent not present? This should only be the case when 'np'
points to the root_node. So in case of !parent I would return -EINVAL
early.
> + return of_device_ensure_probed(of_get_parent(np));
^
parent?
Not related to this patch but the logic would become easier if would
have devices for each mtd-part, like the kernel does. In such case we
could avoid these special handlings and just use
of_device_ensure_probed() at least for the mtd-parts, nvmem-cells still
need a special handling.
Regards,
Marco
> - return np ? of_device_ensure_probed(np) : -EINVAL;
> + if (of_get_compatible_child(np, "fixed-partitions"))
> + return of_device_ensure_probed(np);
> +
> + if (!of_property_present(np, "compatible") ||
> + of_device_is_compatible(np, "nvmem-cells"))
> + return of_device_ensure_probed(parent);
> +
> + return of_device_ensure_probed(np);
> }
> EXPORT_SYMBOL_GPL(of_partition_ensure_probed);
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index ab737629325a..78b8a31331db 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -484,7 +484,7 @@ int of_device_ensure_probed(struct device_node *np)
> {
> struct device *dev;
>
> - if (!deep_probe_is_supported())
> + if (!np || !deep_probe_is_supported())
> return 0;
>
> dev = of_device_create_on_demand(np);
> --
> 2.39.2
>
>
>
RE: [PATCH v2] Porting barebox to a new SoC
Hi Ahmad, Using end of SRAM as PBL stack is currently not working because we need 40bit address (0xC0_0020_). This needs a fix to ENTRY_FUNCTION_WITHSTACK macro. I tried just to change const u32 __stack_top = (stack_top); to const u64 __stack_top = (stack_top); but there were linking errors caused by: ASSERT(. - __pbl_board_stack_top <= 8, "Only One PBL per Image allowed") Regarding the arm timer: Adding the timer entry to DT solved the issue. Regarding the UART: Indeed CONFIG_SERIAL_AMBA_PL011 was set on spider_defconfig (also verified it generated the correct entry on .config). I also noticed that if I remove the putc_ll implementation there is no Tx at all from Barebox. Could it be a hint? Thanks, Lior. > -Original Message- > From: Ahmad Fatoum > Sent: Wednesday, May 31, 2023 11:40 AM > To: Lior Weintraub ; Ahmad Fatoum ; > barebox@lists.infradead.org > Subject: Re: [PATCH v2] Porting barebox to a new SoC > > CAUTION: External Sender > > Hi Lior, > > On 31.05.23 10:05, Lior Weintraub wrote: > > Hi Ahmad, > > > > Thanks again for your prompt reply and accurate tips! > > Took the following changes: > > 1. Increasing the DRAM size to 128MB (let barebox start from 0). > > 2. Set PBL stack to offset 2MB from DRAM > > Just use end of SRAM, so you are completely independent of DRAM > until you call barebox_arm_entry. > > > 3. Fix the device tree "memory" entry to have 128MB. > > (y) > > > 4. Load barebox-spider-mk1-evk.img to SRAM and run from there. > > > > Now I can see the following logs: > > uncompress.c: memory at 0x, size 0x0800 > > uncompress.c: uncompressing barebox binary at 0x00c021c0 > (size 0x0001e3a4) to 0x07e0 (uncompressed size: 0x000390d0) > > uncompress.c: jumping to uncompressed image at 0x07e0 > > start.c: memory at 0x, size 0x0800 > > start.c: found DTB in boarddata, copying to 0x07dffcc0 > > start.c: initializing malloc pool at 0x079ffcc0 (size 0x0040) > > start.c: starting barebox... > > initcall-> command_slice_init+0x0/0x3c > > initcall-> globalvar_init+0x0/0x80 > > register_device: global > > register_device: nv > > initcall-> platform_init+0x0/0x1c > > register_device: platform > > initcall-> amba_bus_init+0x0/0x1c > > register_device: amba > > initcall-> spi_bus_init+0x0/0x1c > > register_device: spi > > initcall-> gpio_desc_alloc+0x0/0x24 > > initcall-> fs_bus_init+0x0/0x1c > > register_device: fs > > initcall-> aarch64_init_vectors+0x0/0x50 > > initcall-> gpio_gate_clock_driver_register+0x0/0x1c > > register_driver: gpio-gate-clock > > initcall-> of_arm_init+0x0/0x5c > > start.c: barebox_arm_boot_dtb: using barebox_boarddata > > using boarddata provided DTB > > adding DT alias:serial0: stem=serial id=0 node=/soc/serial@d000307000 > > register_device: machine > > of_platform_bus_create() - skipping /chosen, no compatible prop > > of_platform_bus_create() - skipping /aliases, no compatible prop > > of_platform_bus_create() - skipping /cpus, no compatible prop > > of_platform_bus_create() - skipping /memory@0, no compatible prop > > of_platform_bus_create() - skipping /soc, no compatible prop > > initcall-> register_autoboot_vars+0x0/0x70 > > initcall-> arm_arch_timer_driver_register+0x0/0x1c > > register_driver: arm-architected-timer > > initcall-> of_timer_init+0x0/0x20 > > initcall-> init_fs+0x0/0x3c > > initcall-> pl011_driver_register+0x0/0x1c > > register_driver: uart-pl011 > > initcall-> of_stdoutpath_init+0x0/0x28 > > initcall-> of_probe_memory+0x0/0x60 > > __request_region ok: 0x:0x07ff flags=0x0 > > initcall-> __bare_init_end+0x0/0x6c > > register_device: mem0 > > initcall-> of_reserved_mem_walk+0x0/0x1ac > > initcall-> mem_malloc_resource+0x0/0xa8 > > __request_region ok: 0x079ffcc0:0x07dffcbf flags=0x200 > > __request_region ok: 0x07e0:0x07e2aeff flags=0x200 > > __request_region ok: 0x07e2af00:0x07e390cf flags=0x200 > > __request_region ok: 0x07e390d0:0x07e3adaf flags=0x200 > > initcall-> bootsource_init+0x0/0x40 > > initcall-> ramfs_init+0x0/0x1c > > register_driver: ramfs > > initcall-> devfs_init+0x0/0x1c > > register_driver: devfs > > initcall-> arm_request_stack+0x0/0x398 > > __request_region ok: 0x07ff:0x07ff7fff flags=0x200 > > initcall-> mount_root+0x0/0x7c > > mount: none on / type ramfs, options= > > register_device: ramfs0 > > probe-> ramfs0 > > mount: none on /dev type devfs, options= > > register_device: devfs0 > > probe-> devfs0 > > initcall-> binfmt_sh_init+0x0/0x1c > > initcall-> binfmt_uimage_init+0x0/0x1c > > initcall-> console_common_init+0x0/0x48 > > initcall-> of_kernel_init+0x0/0x28 > > initcall-> console_ctrlc_init+0x0/0x30 > > initcall-> mem_init+0x0/0x90 > > register_device: mem1 > > register_driver: mem > > probe-> mem0 > > probe-> mem1 > > initcall-> of_partition_init+0x0/0x48 > > initcall-> prng_init+0x0/0x40 > > initcall-> null_init+0x0/0x40 > > initcall-> full_init+0x0/0x40 > > initcall-> zero_init+0x0/0x40 > > initcall->
Re: [PATCH 03/18] cdev: fix for_each_cdev macro
On 23-05-31, Ahmad Fatoum wrote: > The macro parameter 'c' was never used, instead hardcoding cdev. > It worked so far anyway, because all users of for_each_cdev used cdev > as the argument. Fix this. > > Signed-off-by: Ahmad Fatoum Reviewed-by: Marco Felsch > --- > include/driver.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/driver.h b/include/driver.h > index d33e0fcbccc9..e1ee3dc2dd7c 100644 > --- a/include/driver.h > +++ b/include/driver.h > @@ -566,7 +566,7 @@ int cdev_truncate(struct cdev*, size_t size); > loff_t cdev_unallocated_space(struct cdev *cdev); > > extern struct list_head cdev_list; > -#define for_each_cdev(c) \ > +#define for_each_cdev(cdev) \ > list_for_each_entry(cdev, _list, list) > > #define DEVFS_PARTITION_FIXED(1U << 0) > -- > 2.39.2 > > >
[PATCH 04/18] of: partition: support of_partition_ensure_probed on parent device
barebox-state code uses of_partition_ensure_probed to resolve the
backend property. We want to allow backend to point directly at a
storage device instead of a partition. We can't determine whether a DT
device is a storage device though before it's probed, so let's have
of_partition_ensure_probed support either case.
Signed-off-by: Ahmad Fatoum
---
drivers/of/partition.c | 26 ++
drivers/of/platform.c | 2 +-
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index 40c47f554ad2..a70e503cec9e 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -110,14 +110,32 @@ int of_parse_partitions(struct cdev *cdev, struct
device_node *node)
return 0;
}
+/**
+ * of_partition_ensure_probed - ensure a parition is probed
+ * @np: pointer to a partition or to a partitionable device
+ * Unfortunately, there is no completely reliable way
+ * to differentiate partitions from devices prior to
+ * probing, because partitions may also have compatibles.
+ * We only handle nvmem-cells, so anything besides that
+ * is assumed to be a device that should be probed directly.
+ *
+ * Returns zero on success or a negative error code otherwise
+ */
int of_partition_ensure_probed(struct device_node *np)
{
- np = of_get_parent(np);
+ struct device_node *parent = of_get_parent(np);
- if (of_device_is_compatible(np, "fixed-partitions"))
- np = of_get_parent(np);
+ if (parent && of_device_is_compatible(parent, "fixed-partitions"))
+ return of_device_ensure_probed(of_get_parent(np));
- return np ? of_device_ensure_probed(np) : -EINVAL;
+ if (of_get_compatible_child(np, "fixed-partitions"))
+ return of_device_ensure_probed(np);
+
+ if (!of_property_present(np, "compatible") ||
+ of_device_is_compatible(np, "nvmem-cells"))
+ return of_device_ensure_probed(parent);
+
+ return of_device_ensure_probed(np);
}
EXPORT_SYMBOL_GPL(of_partition_ensure_probed);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ab737629325a..78b8a31331db 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -484,7 +484,7 @@ int of_device_ensure_probed(struct device_node *np)
{
struct device *dev;
- if (!deep_probe_is_supported())
+ if (!np || !deep_probe_is_supported())
return 0;
dev = of_device_create_on_demand(np);
--
2.39.2
[PATCH 06/18] driver: add new cdev_is_partition helper
Partitions will have cdev->master != NULL, so often code will just do
if (cdev->master) to check if a cdev is a partition. This is suboptimal
as it may be misinterpreted by readers as meaning that the cdev is the
master device, while it's the other way round.
Let's define cdev_is_partition instead and use it everywhere, where
cdev->master is only checked, but not dereferenced.
Signed-off-by: Ahmad Fatoum
---
drivers/base/driver.c | 2 +-
fs/devfs-core.c | 10 +-
include/driver.h | 4
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index f00be99cdcbf..10d765e1a213 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -277,7 +277,7 @@ int unregister_device(struct device *old_dev)
}
list_for_each_entry_safe(cdev, ct, _dev->cdevs, devices_list) {
- if (cdev->master) {
+ if (cdev_is_partition(cdev)) {
dev_dbg(old_dev, "unregister part %s\n", cdev->name);
devfs_del_partition(cdev->name);
}
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index fbcf68e81597..bb66993b90f9 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -38,7 +38,7 @@ int devfs_partition_complete(struct string_list *sl, char
*instr)
len = strlen(instr);
for_each_cdev(cdev) {
- if (cdev->master &&
+ if (cdev_is_partition(cdev) &&
!strncmp(instr, cdev->name, len)) {
string_list_add_asprintf(sl, "%s ", cdev->name);
}
@@ -101,7 +101,7 @@ struct cdev *cdev_by_partuuid(const char *partuuid)
return NULL;
for_each_cdev(cdev) {
- if (cdev->master && !strcasecmp(cdev->uuid, partuuid))
+ if (cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
partuuid))
return cdev;
}
return NULL;
@@ -115,7 +115,7 @@ struct cdev *cdev_by_diskuuid(const char *diskuuid)
return NULL;
for_each_cdev(cdev) {
- if (!cdev->master && !strcasecmp(cdev->uuid, diskuuid))
+ if (!cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
diskuuid))
return cdev;
}
return NULL;
@@ -393,7 +393,7 @@ int devfs_remove(struct cdev *cdev)
list_for_each_entry_safe(c, tmp, >links, link_entry)
devfs_remove(c);
- if (cdev->master)
+ if (cdev_is_partition(cdev))
list_del(>partition_entry);
if (cdev->link)
@@ -549,7 +549,7 @@ int devfs_del_partition(const char *name)
return ret;
}
- if (!cdev->master)
+ if (!cdev_is_partition(cdev))
return -EINVAL;
if (cdev->flags & DEVFS_PARTITION_FIXED)
return -EPERM;
diff --git a/include/driver.h b/include/driver.h
index e1ee3dc2dd7c..00b4a0e4af75 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -564,6 +564,10 @@ int cdev_discard_range(struct cdev*, loff_t count, loff_t
offset);
int cdev_memmap(struct cdev*, void **map, int flags);
int cdev_truncate(struct cdev*, size_t size);
loff_t cdev_unallocated_space(struct cdev *cdev);
+static inline bool cdev_is_partition(const struct cdev *cdev)
+{
+ return cdev->master != NULL;
+}
extern struct list_head cdev_list;
#define for_each_cdev(cdev) \
--
2.39.2
[PATCH 08/18] cdev: use more descriptive struct cdev::diskuuid/partuuid
The UUID field has different meanings:
For a master cdev:
- GPT Header DiskGUID if GPT-formatted
- MBR Header NT Disk Signature if MBR-formatted
For a partition cdev:
- GPT UniquePartitionGUID
- MBR Header NT Disk Signature followed by "-${partititon_number}"
Later code will add yet another UUID (Partition Type GUID), so let's
make existing code more readable by using either diskuuid or partuuid as
appropriate.
No functional change.
Signed-off-by: Ahmad Fatoum
---
common/bootm.c | 2 +-
common/partitions.c| 2 +-
common/partitions/dos.c| 2 +-
common/partitions/efi.c| 4 ++--
drivers/misc/storage-by-uuid.c | 4 ++--
fs/devfs-core.c| 4 ++--
fs/fs.c| 9 +
include/driver.h | 10 +-
8 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c
index 91a6e1688674..791d6b8fbbf1 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -734,7 +734,7 @@ int bootm_boot(struct bootm_data *bootm_data)
if (!root_cdev)
pr_err("no cdev found for %s, cannot
set root= option\n",
root_dev_name);
- else if (!root_cdev->uuid[0])
+ else if (!root_cdev->partuuid[0])
pr_err("%s doesn't have a PARTUUID,
cannot set root= option\n",
root_dev_name);
}
diff --git a/common/partitions.c b/common/partitions.c
index 9cca5c4a1546..b579559672a0 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -51,7 +51,7 @@ static int register_one_partition(struct block_device *blk,
cdev->flags |= DEVFS_PARTITION_FROM_TABLE;
cdev->dos_partition_type = part->dos_partition_type;
- strcpy(cdev->uuid, part->partuuid);
+ strcpy(cdev->partuuid, part->partuuid);
free(partition_name);
diff --git a/common/partitions/dos.c b/common/partitions/dos.c
index 566c8dd949b4..ad60c0b27b46 100644
--- a/common/partitions/dos.c
+++ b/common/partitions/dos.c
@@ -183,7 +183,7 @@ static void dos_partition(void *buf, struct block_device
*blk,
uint32_t signature = get_unaligned_le32(buf + 0x1b8);
if (signature)
- sprintf(blk->cdev.uuid, "%08x", signature);
+ sprintf(blk->cdev.diskuuid, "%08x", signature);
table = (struct partition_entry *)[446];
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index a6c95f3969d1..780a8695e8a8 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -446,8 +446,8 @@ static void efi_partition(void *buf, struct block_device
*blk,
return;
}
- snprintf(blk->cdev.uuid, sizeof(blk->cdev.uuid), "%pUl",
>disk_guid);
- dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.uuid);
+ snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl",
>disk_guid);
+ dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
nb_part = le32_to_cpu(gpt->num_partition_entries);
diff --git a/drivers/misc/storage-by-uuid.c b/drivers/misc/storage-by-uuid.c
index a938bfaaa2c4..a7a66a1421a3 100644
--- a/drivers/misc/storage-by-uuid.c
+++ b/drivers/misc/storage-by-uuid.c
@@ -140,8 +140,8 @@ static void check_exist(struct sbu *sbu)
struct cdev *cdev;
for_each_cdev(cdev) {
- if (!strcmp(cdev->uuid, sbu->uuid)) {
- dev_dbg(sbu->dev, "Found %s %s\n", cdev->name,
cdev->uuid);
+ if (!strcmp(cdev->diskuuid, sbu->uuid)) {
+ dev_dbg(sbu->dev, "Found %s %s\n", cdev->name,
cdev->diskuuid);
storage_by_uuid_add_partitions(sbu, cdev);
}
}
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index bb66993b90f9..a0732dafca42 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -101,7 +101,7 @@ struct cdev *cdev_by_partuuid(const char *partuuid)
return NULL;
for_each_cdev(cdev) {
- if (cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
partuuid))
+ if (cdev_is_partition(cdev) && !strcasecmp(cdev->partuuid,
partuuid))
return cdev;
}
return NULL;
@@ -115,7 +115,7 @@ struct cdev *cdev_by_diskuuid(const char *diskuuid)
return NULL;
for_each_cdev(cdev) {
- if (!cdev_is_partition(cdev) && !strcasecmp(cdev->uuid,
diskuuid))
+ if (!cdev_is_partition(cdev) && !strcasecmp(cdev->diskuuid,
diskuuid))
return cdev;
}
return NULL;
diff --git a/fs/fs.c b/fs/fs.c
index ba60766a065a..1820e48393af 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -104,8 +104,9 @@ void cdev_print(const struct cdev *cdev)
nbytes +=
[PATCH 10/18] cdev: have devfs_add_partition return existing identical partition, not NULL
Starting with commit 7f9f45b9bfef ("devfs: Do not create overlapping
partitions"), any overlapping is disallowed. Overlapping can be useful
though to bridge the gap between partition described in DT and via
on-disk partition tables. Let's handle the case of identical partitions
specially and have it neither be an error or a duplicate partition, but
instead just return the existing partition. This existing partition will
be given a device tree node and thus enabling schemes like:
&{/state} {
backend = <_part>;
};
{
partitions {
compatible = "fixed-partitions";
#address-cells = <2>;
#size-cells = <2>;
state_part: partition@530 {
label = "barebox-state";
/* will be folded with overlapping GPT partition if
found */
reg = <0x0 0x530 0x0 0x10>;
};
};
};
Signed-off-by: Ahmad Fatoum
---
fs/devfs-core.c | 50 ++---
1 file changed, 39 insertions(+), 11 deletions(-)
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index a0732dafca42..b3a274d01ee0 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -402,6 +402,12 @@ int devfs_remove(struct cdev *cdev)
return 0;
}
+static bool region_identical(loff_t starta, loff_t lena,
+loff_t startb, loff_t lenb)
+{
+ return starta == startb && lena == lenb;
+}
+
static bool region_overlap(loff_t starta, loff_t lena,
loff_t startb, loff_t lenb)
{
@@ -412,10 +418,22 @@ static bool region_overlap(loff_t starta, loff_t lena,
return 1;
}
-static int check_overlap(struct cdev *cdev, const char *name, loff_t offset,
loff_t size)
+/**
+ * check_overlap() - check overlap with existing partitions
+ * @cdev: parent cdev
+ * @name: partition name for informational purposes on conflict
+ * @offset: offset of new partition to be added
+ * @size: size of new partition to be added
+ *
+ * Return: NULL if no overlapping partition found or overlapping
+ * partition if and only if it's identical in offset and size
+ * to an existing partition. Otherwise, PTR_ERR(-EINVAL).
+ */
+static struct cdev *check_overlap(struct cdev *cdev, const char *name, loff_t
offset, loff_t size)
{
struct cdev *cpart;
loff_t cpart_offset;
+ int ret;
list_for_each_entry(cpart, >partitions, partition_entry) {
cpart_offset = cpart->offset;
@@ -428,20 +446,28 @@ static int check_overlap(struct cdev *cdev, const char
*name, loff_t offset, lof
if (cpart->mtd)
cpart_offset = cpart->mtd->master_offset;
- if (region_overlap(cpart_offset, cpart->size,
- offset, size))
+ if (region_identical(cpart_offset, cpart->size, offset, size)) {
+ ret = 0;
goto conflict;
+ }
+
+ if (region_overlap(cpart_offset, cpart->size, offset, size)) {
+ ret = -EINVAL;
+ goto conflict;
+ }
}
- return 0;
+ return NULL;
conflict:
- pr_err("New partition %s (0x%08llx-0x%08llx) on %s "
- "overlaps with partition %s (0x%08llx-0x%08llx), not creating
it\n",
- name, offset, offset + size - 1, cdev->name,
- cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
+ __pr_printk(ret ? MSG_WARNING : MSG_DEBUG,
+ "New partition %s (0x%08llx-0x%08llx) on %s "
+ "%s with partition %s (0x%08llx-0x%08llx), not creating
it\n",
+ name, offset, offset + size - 1, cdev->name,
+ ret ? "conflicts" : "identical",
+ cpart->name, cpart_offset, cpart_offset + cpart->size - 1);
- return -EINVAL;
+ return ret ? ERR_PTR(ret) : cpart;
}
static struct cdev *__devfs_add_partition(struct cdev *cdev,
@@ -449,6 +475,7 @@ static struct cdev *__devfs_add_partition(struct cdev *cdev,
{
loff_t offset, size;
static struct cdev *new;
+ struct cdev *overlap;
if (cdev_by_name(partinfo->name))
return ERR_PTR(-EEXIST);
@@ -479,8 +506,9 @@ static struct cdev *__devfs_add_partition(struct cdev *cdev,
return ERR_PTR(-EINVAL);
}
- if (check_overlap(cdev, partinfo->name, offset, size))
- return ERR_PTR(-EINVAL);
+ overlap = check_overlap(cdev, partinfo->name, offset, size);
+ if (overlap)
+ return overlap;
if (IS_ENABLED(CONFIG_MTD) && cdev->mtd) {
struct mtd_info *mtd;
--
2.39.2
[PATCH 01/18] common: partitions: decouple from EFI GUID definition
We have three UUID/GUID definitions in barebox:
- efi_guid_t: for EFI GUIDs
- uuid_t: for RFC 4122/DCE 1.1 (Variant 1) UUIDs
- guid_t: Apparently UUIDs stored in little-endian
In preparation for switching efi_guid_t to be a special case of guid_t
like in Linux, let's replace non-EFI uses of efi_guid_t with guid_t.
This allows us to drop the efi.h header outside of EFI code.
This also involves two replacements of efi_char16_t with wchar_t.
This is ok, as we always build with -fshort-wchar.
Signed-off-by: Ahmad Fatoum
---
common/partitions/efi.c| 4 ++--
common/partitions/parser.h | 1 +
include/efi/partition.h| 27 ++-
lib/Makefile | 1 +
lib/uuid.c | 15 +++
5 files changed, 33 insertions(+), 15 deletions(-)
create mode 100644 lib/uuid.c
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index ffdbd9a56f85..a6c95f3969d1 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -232,7 +232,7 @@ static int is_gpt_valid(struct block_device *blk, u64 lba,
static inline int
is_pte_valid(const gpt_entry *pte, const u64 lastlba)
{
- if ((!efi_guidcmp(pte->partition_type_guid, EFI_NULL_GUID)) ||
+ if (guid_is_null(>partition_type_guid) ||
le64_to_cpu(pte->starting_lba) > lastlba ||
le64_to_cpu(pte->ending_lba) > lastlba)
return 0;
@@ -287,7 +287,7 @@ compare_gpts(struct device *dev, gpt_header *pgpt,
gpt_header *agpt,
(unsigned long long)le64_to_cpu(agpt->last_usable_lba));
error_found++;
}
- if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
+ if (!guid_equal(>disk_guid, >disk_guid)) {
dev_warn(dev, "GPT:disk_guids don't match.\n");
error_found++;
}
diff --git a/common/partitions/parser.h b/common/partitions/parser.h
index d67f8e1d6a09..f2f692f7903b 100644
--- a/common/partitions/parser.h
+++ b/common/partitions/parser.h
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
#define MAX_PARTITION 128
diff --git a/include/efi/partition.h b/include/efi/partition.h
index a9b10c126654..0ca2a72eb9b7 100644
--- a/include/efi/partition.h
+++ b/include/efi/partition.h
@@ -21,7 +21,8 @@
#ifndef FS_PART_EFI_H_INCLUDED
#define FS_PART_EFI_H_INCLUDED
-#include
+#include
+#include
#define MSDOS_MBR_SIGNATURE 0xaa55
#define EFI_PMBR_OSTYPE_EFI 0xEF
@@ -33,25 +34,25 @@
#define GPT_PRIMARY_PARTITION_TABLE_LBA 1
#define PARTITION_SYSTEM_GUID \
- EFI_GUID( 0xC12A7328, 0xF81F, 0x11d2, \
+ GUID_INIT( 0xC12A7328, 0xF81F, 0x11d2, \
0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
#define LEGACY_MBR_PARTITION_GUID \
- EFI_GUID( 0x024DEE41, 0x33E7, 0x11d3, \
+ GUID_INIT( 0x024DEE41, 0x33E7, 0x11d3, \
0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
#define PARTITION_MSFT_RESERVED_GUID \
- EFI_GUID( 0xE3C9E316, 0x0B5C, 0x4DB8, \
+ GUID_INIT( 0xE3C9E316, 0x0B5C, 0x4DB8, \
0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
#define PARTITION_BASIC_DATA_GUID \
- EFI_GUID( 0xEBD0A0A2, 0xB9E5, 0x4433, \
+ GUID_INIT( 0xEBD0A0A2, 0xB9E5, 0x4433, \
0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
#define PARTITION_LINUX_RAID_GUID \
- EFI_GUID( 0xa19d880f, 0x05fc, 0x4d3b, \
+ GUID_INIT( 0xa19d880f, 0x05fc, 0x4d3b, \
0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
#define PARTITION_LINUX_SWAP_GUID \
- EFI_GUID( 0x0657fd6d, 0xa4ab, 0x43c4, \
+ GUID_INIT( 0x0657fd6d, 0xa4ab, 0x43c4, \
0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
#define PARTITION_LINUX_LVM_GUID \
- EFI_GUID( 0xe6d6d379, 0xf507, 0x44c2, \
+ GUID_INIT( 0xe6d6d379, 0xf507, 0x44c2, \
0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
/* based on linux/include/genhd.h */
@@ -79,7 +80,7 @@ typedef struct _gpt_header {
__le64 alternate_lba;
__le64 first_usable_lba;
__le64 last_usable_lba;
- efi_guid_t disk_guid;
+ guid_t disk_guid;
__le64 partition_entry_lba;
__le32 num_partition_entries;
__le32 sizeof_partition_entry;
@@ -98,14 +99,14 @@ typedef struct _gpt_entry_attributes {
u64 type_guid_specific:16;
} __attribute__ ((packed)) gpt_entry_attributes;
-#define GPT_PARTNAME_MAX_SIZE (72 / sizeof (efi_char16_t))
+#define GPT_PARTNAME_MAX_SIZE (72 / sizeof (wchar_t))
typedef struct _gpt_entry {
- efi_guid_t partition_type_guid;
- efi_guid_t unique_partition_guid;
+ guid_t partition_type_guid;
+ guid_t unique_partition_guid;
__le64 starting_lba;
__le64 ending_lba;
gpt_entry_attributes attributes;
- efi_char16_t partition_name[GPT_PARTNAME_MAX_SIZE];
+ wchar_t partition_name[GPT_PARTNAME_MAX_SIZE];
} __attribute__ ((packed)) gpt_entry;
typedef struct
[PATCH 18/18] state: allow lookup of barebox state partition by Type GUID
The backend device tree property so far always pointed at a partition.
Let's allow pointing it at GPT storage devices directly and lookup
the correct barebox state partition by the well-known type GUID:
4778ed65-bf42-45fa-9c5b-287a1dc4aab1
Signed-off-by: Ahmad Fatoum
---
common/state/state.c | 22 ++
include/driver.h | 17 +
include/state.h | 4
3 files changed, 43 insertions(+)
diff --git a/common/state/state.c b/common/state/state.c
index 88e246198fb8..8f56c60b0e82 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -21,8 +21,10 @@
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
@@ -595,6 +597,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t
*offset, size_t *size)
}
#endif
+static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -641,6 +645,24 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
+ /* Is the backend referencing an on-disk partitonable block device? */
+ if (cdev_is_block_disk(cdev)) {
+ struct cdev *partcdev = NULL;
+
+ if (cdev_is_gpt_partitioned(cdev))
+ partcdev = cdev_find_child_by_typeuuid(cdev,
_state_partition_guid);
+
+ if (!partcdev) {
+ ret = -EINVAL;
+ goto out_release_state;
+ }
+
+ pr_debug("%s: backend GPT partition looked up via
PartitionTypeGUID\n",
+node->full_name);
+
+ cdev = partcdev;
+ }
+
state->backend_path = cdev_to_devpath(cdev, , );
pr_debug("%s: backend resolved to %s\n", node->full_name,
state->backend_path);
diff --git a/include/driver.h b/include/driver.h
index 6407f7d6ba36..579b03fbac34 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -585,6 +585,23 @@ extern struct list_head cdev_list;
#define for_each_cdev(cdev) \
list_for_each_entry(cdev, _list, list)
+#define for_each_cdev_partition(partcdev, cdev) \
+ list_for_each_entry((partcdev), &(cdev)->partitions, partition_entry)
+
+
+static inline struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev,
+ guid_t *typeuuid)
+{
+ struct cdev *partcdev;
+
+ for_each_cdev_partition(partcdev, cdev) {
+ if (guid_equal(>typeuuid, typeuuid))
+ return partcdev;
+ }
+
+ return NULL;
+}
+
#define DEVFS_PARTITION_FIXED (1U << 0)
#define DEVFS_PARTITION_READONLY (1U << 1)
#define DEVFS_IS_CHARACTER_DEV (1U << 3)
diff --git a/include/state.h b/include/state.h
index bffcd5a9007f..3daf82c0735f 100644
--- a/include/state.h
+++ b/include/state.h
@@ -62,4 +62,8 @@ static inline int state_read_mac(struct state *state, const
char *name, u8 *buf)
#endif /* #if IS_ENABLED(CONFIG_STATE) / #else */
+#define BAREBOX_STATE_PARTITION_GUID \
+ GUID_INIT(0x4778ed65, 0xbf42, 0x45fa, 0x9c, 0x5b, \
+0x28, 0x7a, 0x1d, 0xc4, 0xaa, 0xb1)
+
#endif /* __STATE_H */
--
2.39.2
[PATCH 02/18] efi: define efi_guid_t as 32-bit aligned guid_t
Let's sync definition with Linux, so we are able to pass efi_guid_t
types to function accepting guid_t. This has the added benefit of us
starting to observe alignment, which may become relevant with barebox
EFI on non-x86.
Signed-off-by: Ahmad Fatoum
---
include/efi.h | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/efi.h b/include/efi.h
index 3595cf05ccb7..1904caf3a4b6 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -14,6 +14,7 @@
*/
#include
#include
+#include
#ifdef CONFIG_EFI_BOOTUP
#define EFIAPI __attribute__((ms_abi))
@@ -66,10 +67,20 @@ typedef u16 efi_char16_t; /* UNICODE character */
typedef u64 efi_physical_addr_t;
typedef void *efi_handle_t;
-
-typedef struct {
- u8 b[16];
-} efi_guid_t;
+/*
+ * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
+ * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
+ * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
+ * this means that firmware services invoked by the kernel may assume that
+ * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
+ * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
+ *
+ * Note that the UEFI spec as well as some comments in the EDK2 code base
+ * suggest that EFI_GUID should be 64-bit aligned, but this appears to be
+ * a mistake, given that no code seems to exist that actually enforces that
+ * or relies on it.
+ */
+typedef guid_t efi_guid_t __aligned(__alignof__(u32));
#define EFI_GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
((efi_guid_t) \
--
2.39.2
[PATCH 00/18] state: allow lookup of barebox state partition by Type GUID
So far, we had basically three ways to reference barebox,state on block devices: - On platforms with device tree, we point at a fixed partition described in the DT - On platforms without device tree, we have a custom binding that does a global lookup by partuuid (or a more obscure one by diskuuid). Both are less than optimal. The first clashes with the need to use a GPT partition for the state and the second doesn't allow relating the UUID with an actual device for the case where e.g. both an SD-Card and an eMMC are flashed with the same image. This series fixes that by: - Supporting definition of fixed partitions that are identical to GPT/MBR partitions. - Alternatively, support pointing barebox-state backend at a GPT-partitioned device and the partition with the barebox-state Type GUID will be taken. Ahmad Fatoum (18): common: partitions: decouple from EFI GUID definition efi: define efi_guid_t as 32-bit aligned guid_t cdev: fix for_each_cdev macro of: partition: support of_partition_ensure_probed on parent device of: of_path: always call of_partition_ensure_probed before resolving driver: add new cdev_is_partition helper commands: stat: remove code duplication for type info cdev: use more descriptive struct cdev::diskuuid/partuuid cdev: record whether partition is parsed from OF cdev: have devfs_add_partition return existing identical partition, not NULL block: parse partition table on block device registration common: partitions: record whether disk is GPT or MBR partitioned block: add cdev_is_block_(device,partition,disk) helpers of: export new of_cdev_find helper state: factor device path lookup into helper function cdev: use cdev::dos_partition_type only if cdev_is_mbr_partitioned common: partitions: efi: record type UUID in cdev state: allow lookup of barebox state partition by Type GUID arch/sandbox/board/hostfile.c | 4 --- common/block.c | 6 common/blspec.c| 2 +- common/bootm.c | 2 +- common/partitions.c| 4 +-- common/partitions/dos.c| 4 ++- common/partitions/efi.c| 11 +++--- common/partitions/parser.h | 6 +++- common/state/state.c | 56 +-- drivers/ata/disk_ata_drive.c | 5 --- drivers/base/driver.c | 2 +- drivers/block/efi-block-io.c | 9 + drivers/block/virtio_blk.c | 8 + drivers/mci/mci-core.c | 6 drivers/misc/storage-by-uuid.c | 4 +-- drivers/nvme/host/core.c | 5 --- drivers/of/of_path.c | 61 +++--- drivers/of/partition.c | 31 + drivers/of/platform.c | 2 +- drivers/usb/storage/usb.c | 5 --- fs/devfs-core.c| 60 - fs/fs.c| 33 -- include/block.h| 15 + include/driver.h | 58 +--- include/efi.h | 19 --- include/efi/partition.h| 27 +++ include/of.h | 1 + include/state.h| 4 +++ lib/Makefile | 1 + lib/uuid.c | 15 + 30 files changed, 328 insertions(+), 138 deletions(-) create mode 100644 lib/uuid.c -- 2.39.2
[PATCH 11/18] block: parse partition table on block device registration
Every instance where we register a block device, it's followed by an
attempt to parse the partition table, most often with a warning when
it fails. Thus let's move partition table parsing into
blockdevice_register.
Signed-off-by: Ahmad Fatoum
---
arch/sandbox/board/hostfile.c | 4
common/block.c| 6 ++
drivers/ata/disk_ata_drive.c | 5 -
drivers/block/efi-block-io.c | 9 +
drivers/block/virtio_blk.c| 8 +---
drivers/mci/mci-core.c| 6 --
drivers/nvme/host/core.c | 5 -
drivers/usb/storage/usb.c | 5 -
8 files changed, 8 insertions(+), 40 deletions(-)
diff --git a/arch/sandbox/board/hostfile.c b/arch/sandbox/board/hostfile.c
index d0f400787d7a..a1ab06b87770 100644
--- a/arch/sandbox/board/hostfile.c
+++ b/arch/sandbox/board/hostfile.c
@@ -166,10 +166,6 @@ static int hf_probe(struct device *dev)
if (err)
return err;
- err = parse_partition_table(>blk);
- if (err)
- dev_warn(dev, "No partition table found\n");
-
dev_info(dev, "registered as block device\n");
} else {
cdev->name = np->name;
diff --git a/common/block.c b/common/block.c
index c39269d3a692..98adcfdf3dab 100644
--- a/common/block.c
+++ b/common/block.c
@@ -6,6 +6,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -408,6 +409,11 @@ int blockdevice_register(struct block_device *blk)
cdev_create_default_automount(>cdev);
+ /* Lack of partition table is unusual, but not a failure */
+ ret = parse_partition_table(blk);
+ if (ret)
+ dev_warn(blk->dev, "No partition table found\n");
+
return 0;
}
diff --git a/drivers/ata/disk_ata_drive.c b/drivers/ata/disk_ata_drive.c
index c1c736a0a88a..2d97710b827a 100644
--- a/drivers/ata/disk_ata_drive.c
+++ b/drivers/ata/disk_ata_drive.c
@@ -254,11 +254,6 @@ static int ata_port_init(struct ata_port *port)
dev_info(dev, "registered /dev/%s\n", port->blk.cdev.name);
- /* create partitions on demand */
- rc = parse_partition_table(>blk);
- if (rc != 0)
- dev_warn(dev, "No partition table found\n");
-
return 0;
on_error:
diff --git a/drivers/block/efi-block-io.c b/drivers/block/efi-block-io.c
index eb4981e86298..7162106ab8ea 100644
--- a/drivers/block/efi-block-io.c
+++ b/drivers/block/efi-block-io.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -184,16 +183,10 @@ static int efi_bio_probe(struct efi_device *efidev)
priv->media_id = media->media_id;
- ret = blockdevice_register(>blk);
- if (ret)
- return ret;
-
if (efi_get_bootsource() == efidev)
bootsource_set_raw_instance(instance);
- parse_partition_table(>blk);
-
- return 0;
+ return blockdevice_register(>blk);
}
static struct efi_driver efi_bio_driver = {
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 660f3a7b6b9b..11e52d9e6457 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -105,13 +105,7 @@ static int virtio_blk_probe(struct virtio_device *vdev)
priv->blk.num_blocks = cap;
priv->blk.ops = _blk_ops;
- ret = blockdevice_register(>blk);
- if (ret)
- return ret;
-
- parse_partition_table(>blk);
-
- return 0;
+ return blockdevice_register(>blk);
}
static void virtio_blk_remove(struct virtio_device *vdev)
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 6d0d6473770c..32edd5382386 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1900,12 +1900,6 @@ static int mci_register_partition(struct mci_part *part)
return 0;
}
- rc = parse_partition_table(>blk);
- if (rc != 0) {
- /* Lack of partition table is unusual, but not a failure */
- dev_warn(>dev, "No partition table found\n");
- }
-
if (np) {
of_parse_partitions(>blk.cdev, np);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index bf9176ce0922..79a5f9325ef8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
#include
-#include
#include "nvme.h"
@@ -373,10 +372,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned
nsid)
goto out_free_id;
}
- ret = parse_partition_table(>blk);
- if (ret)
- dev_warn(ctrl->dev, "No partition table found\n");
-
return;
out_free_id:
kfree(id);
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 103ae293a3a4..dda713196071 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -420,11 +420,6 @@ static int usb_stor_add_blkdev(struct us_data *us,
[PATCH 17/18] common: partitions: efi: record type UUID in cdev
We already record DOS partition type in cdev, so let's do the same for
GPT Type UUID. This will be used in a later commit to identify
barebox-state partitions.
Signed-off-by: Ahmad Fatoum
---
common/partitions.c| 2 +-
common/partitions/efi.c| 1 +
common/partitions/parser.h | 5 -
fs/fs.c| 2 ++
include/driver.h | 6 +-
5 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/common/partitions.c b/common/partitions.c
index b579559672a0..e3e8a9f3044d 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -50,7 +50,7 @@ static int register_one_partition(struct block_device *blk,
cdev->flags |= DEVFS_PARTITION_FROM_TABLE;
- cdev->dos_partition_type = part->dos_partition_type;
+ cdev->typeuuid = part->typeuuid;
strcpy(cdev->partuuid, part->partuuid);
free(partition_name);
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index df63b82afe24..2756337ab284 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -471,6 +471,7 @@ static void efi_partition(void *buf, struct block_device
*blk,
pentry->size++;
part_set_efi_name([i], pentry->name);
snprintf(pentry->partuuid, sizeof(pentry->partuuid), "%pUl",
[i].unique_partition_guid);
+ pentry->typeuuid = ptes[i].partition_type_guid;
pd->used_entries++;
}
}
diff --git a/common/partitions/parser.h b/common/partitions/parser.h
index f2f692f7903b..9cc41a7573fe 100644
--- a/common/partitions/parser.h
+++ b/common/partitions/parser.h
@@ -17,10 +17,13 @@
struct partition {
char name[MAX_PARTITION_NAME];
- u8 dos_partition_type;
char partuuid[MAX_UUID_STR];
uint64_t first_sec;
uint64_t size;
+ union {
+ u8 dos_partition_type;
+ guid_t typeuuid;
+ };
};
struct partition_desc {
diff --git a/fs/fs.c b/fs/fs.c
index 9a92e6e251e5..16cc072adfaf 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -110,6 +110,8 @@ void cdev_print(const struct cdev *cdev)
nbytes += printf("Filetype: %s\t",
file_type_to_string(cdev->filetype));
if (cdev_is_mbr_partitioned(cdev->master))
nbytes += printf("DOS parttype: 0x%02x\t",
cdev->dos_partition_type);
+ else if (cdev_is_gpt_partitioned(cdev->master))
+ nbytes += printf("GPT typeuuid: %pUl\t", >typeuuid);
if (*cdev->partuuid || *cdev->diskuuid)
nbytes += printf("%sUUID: %s", cdev_is_partition(cdev) ? "PART"
: "DISK",
cdev_is_partition(cdev) ? cdev->partuuid :
cdev->diskuuid);
diff --git a/include/driver.h b/include/driver.h
index 5f2eae65466f..6407f7d6ba36 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
@@ -536,12 +537,15 @@ struct cdev {
unsigned int flags;
int open;
struct mtd_info *mtd;
- u8 dos_partition_type;
struct cdev *link;
struct list_head link_entry, links;
struct list_head partition_entry, partitions;
struct cdev *master;
enum filetype filetype;
+ union {
+ u8 dos_partition_type;
+ guid_t typeuuid;
+ };
};
int devfs_create(struct cdev *);
--
2.39.2
[PATCH 05/18] of: of_path: always call of_partition_ensure_probed before resolving
of_find_path may be called on a partition, whose parent device is not
yet probed. state code solves that by calling of_partition_ensure_probed
before of_find_path_by_nde, but really we should be doing that for all
calls to of_find_path. Do so.
Signed-off-by: Ahmad Fatoum
---
common/state/state.c | 4
drivers/of/of_path.c | 2 ++
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/common/state/state.c b/common/state/state.c
index 6b4acbb32bcc..11cc86ff73be 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -618,10 +618,6 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
}
#ifdef __BAREBOX__
- ret = of_partition_ensure_probed(partition_node);
- if (ret)
- goto out_release_state;
-
ret = of_find_path_by_node(partition_node, >backend_path, 0);
#else
ret = of_get_devicepath(partition_node, >backend_path, ,
);
diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
index 1268cf36ee5b..059690e9b8e8 100644
--- a/drivers/of/of_path.c
+++ b/drivers/of/of_path.c
@@ -43,6 +43,8 @@ static int __of_find_path(struct device_node *node, const
char *part, char **out
struct cdev *cdev;
bool add_bb = false;
+ of_partition_ensure_probed(node);
+
dev = of_find_device_by_node_path(node->full_name);
if (!dev) {
int ret;
--
2.39.2
[PATCH 12/18] common: partitions: record whether disk is GPT or MBR partitioned
Currently, the only way to differentiate between a GPT disk and a MBR
one is to check whether the cdev's device has a guid (=> GPT) or a
nt_signature (=> MBR) device parameter. We already have a flag parameter
though, so let's record this info there for easy retrieval.
We intentionally don't use the struct cdev::filetype member, because
we don't want to change behavior of file_detect_type().
Signed-off-by: Ahmad Fatoum
---
common/partitions/dos.c | 2 ++
common/partitions/efi.c | 2 ++
fs/fs.c | 4
include/driver.h| 20 +---
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/common/partitions/dos.c b/common/partitions/dos.c
index ad60c0b27b46..7472824b00b9 100644
--- a/common/partitions/dos.c
+++ b/common/partitions/dos.c
@@ -185,6 +185,8 @@ static void dos_partition(void *buf, struct block_device
*blk,
if (signature)
sprintf(blk->cdev.diskuuid, "%08x", signature);
+ blk->cdev.flags |= DEVFS_IS_MBR_PARTITIONED;
+
table = (struct partition_entry *)[446];
for (i = 0; i < 4; i++) {
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index 780a8695e8a8..df63b82afe24 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -449,6 +449,8 @@ static void efi_partition(void *buf, struct block_device
*blk,
snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl",
>disk_guid);
dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
+ blk->cdev.flags |= DEVFS_IS_GPT_PARTITIONED;
+
nb_part = le32_to_cpu(gpt->num_partition_entries);
if (nb_part > MAX_PARTITION) {
diff --git a/fs/fs.c b/fs/fs.c
index 9d8aab268ca4..2d2d327c5fbc 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -94,6 +94,10 @@ void cdev_print(const struct cdev *cdev)
printf(" table-partition");
if (cdev->flags & DEVFS_IS_MCI_MAIN_PART_DEV)
printf(" mci-main-partition");
+ if (cdev->flags & DEVFS_IS_GPT_PARTITIONED)
+ printf(" gpt-partitioned");
+ if (cdev->flags & DEVFS_IS_MBR_PARTITIONED)
+ printf(" mbr-partitioned");
if (cdev->mtd)
printf(" mtd");
printf(" )");
diff --git a/include/driver.h b/include/driver.h
index 118d2adb6750..5f2eae65466f 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -584,9 +584,23 @@ extern struct list_head cdev_list;
#define DEVFS_PARTITION_FIXED (1U << 0)
#define DEVFS_PARTITION_READONLY (1U << 1)
#define DEVFS_IS_CHARACTER_DEV (1U << 3)
-#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 4)
-#define DEVFS_PARTITION_FROM_OF(1U << 5)
-#define DEVFS_PARTITION_FROM_TABLE (1U << 6)
+#define DEVFS_PARTITION_FROM_OF(1U << 4)
+#define DEVFS_PARTITION_FROM_TABLE (1U << 5)
+#define DEVFS_IS_GPT_PARTITIONED (1U << 6)
+#define DEVFS_IS_MBR_PARTITIONED (1U << 7)
+#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 8)
+
+static inline bool cdev_is_gpt_partitioned(const struct cdev *master)
+{
+ return master && (master->flags & DEVFS_IS_GPT_PARTITIONED)
+ == DEVFS_IS_GPT_PARTITIONED;
+}
+
+static inline bool cdev_is_mbr_partitioned(const struct cdev *master)
+{
+ return master && (master->flags & DEVFS_IS_MBR_PARTITIONED)
+ == DEVFS_IS_MBR_PARTITIONED;
+}
struct cdev *devfs_add_partition(const char *devname, loff_t offset,
loff_t size, unsigned int flags, const char *name);
--
2.39.2
[PATCH 16/18] cdev: use cdev::dos_partition_type only if cdev_is_mbr_partitioned
dos_partition_type == 0 can mean that either a partition is not
a MBR partition or that it indeed has a partition type of 0x00.
In preparation for using that field in a union, explicitly check if we
have a MBR partition.
Signed-off-by: Ahmad Fatoum
---
common/blspec.c | 2 +-
fs/fs.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/blspec.c b/common/blspec.c
index e95a8dba8d76..8c7970da8915 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -729,7 +729,7 @@ int blspec_scan_device(struct bootentries *bootentries,
struct device *dev)
* partition with the MBR type id of 0xEA already exists it
* should be used as $BOOT
*/
- if (cdev->dos_partition_type == 0xea) {
+ if (cdev_is_mbr_partitioned(cdev->master) &&
cdev->dos_partition_type == 0xea) {
ret = blspec_scan_cdev(bootentries, cdev);
if (ret == 0)
ret = -ENOENT;
diff --git a/fs/fs.c b/fs/fs.c
index 2d2d327c5fbc..9a92e6e251e5 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -108,7 +108,7 @@ void cdev_print(const struct cdev *cdev)
if (cdev->filetype)
nbytes += printf("Filetype: %s\t",
file_type_to_string(cdev->filetype));
- if (cdev->dos_partition_type)
+ if (cdev_is_mbr_partitioned(cdev->master))
nbytes += printf("DOS parttype: 0x%02x\t",
cdev->dos_partition_type);
if (*cdev->partuuid || *cdev->diskuuid)
nbytes += printf("%sUUID: %s", cdev_is_partition(cdev) ? "PART"
: "DISK",
--
2.39.2
[PATCH 14/18] of: export new of_cdev_find helper
__of_find_path goes throught the hassle of determining the cdev, only to
discard it again and return either zero or an error code.
Follow up commits will need to get the cdev corresponding to a path in
the DT. So let's make that easier by exporting a suitable helper function.
Signed-off-by: Ahmad Fatoum
---
drivers/of/of_path.c | 59 ++--
include/of.h | 1 +
2 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/drivers/of/of_path.c b/drivers/of/of_path.c
index 059690e9b8e8..4d9f7b6005af 100644
--- a/drivers/of/of_path.c
+++ b/drivers/of/of_path.c
@@ -27,21 +27,17 @@ struct device *of_find_device_by_node_path(const char *path)
}
/**
- * __of_find_path
+ * __of_cdev_find
*
* @node: The node to find the cdev for, can be the device or a
*partition in the device
* @part: Optionally, a description of a partition of @node. See of_find_path
- * @outpath: if this function returns 0 outpath will contain the path belonging
- * to the input path description. Must be freed with free().
- * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
*
*/
-static int __of_find_path(struct device_node *node, const char *part, char
**outpath, unsigned flags)
+static struct cdev *__of_cdev_find(struct device_node *node, const char *part)
{
struct device *dev;
struct cdev *cdev;
- bool add_bb = false;
of_partition_ensure_probed(node);
@@ -56,24 +52,17 @@ static int __of_find_path(struct device_node *node, const
char *part, char **out
/* when partuuid is specified short-circuit the search
for the cdev */
ret = of_property_read_string(node, "partuuid", );
- if (!ret) {
- cdev = cdev_by_partuuid(uuid);
- if (!cdev)
- return -ENODEV;
-
- *outpath = basprintf("/dev/%s", cdev->name);
-
- return 0;
- }
+ if (!ret)
+ return cdev_by_partuuid(uuid) ?:
ERR_PTR(-ENODEV);
}
dev = of_find_device_by_node_path(devnode->full_name);
if (!dev)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
}
if (dev->bus && !dev->driver)
- return -EPROBE_DEFER;
+ return ERR_PTR(-EPROBE_DEFER);
device_detect(dev);
@@ -82,8 +71,40 @@ static int __of_find_path(struct device_node *node, const
char *part, char **out
else
cdev = cdev_by_device_node(node);
- if (!cdev)
- return -ENOENT;
+ return cdev ?: ERR_PTR(-ENOENT);
+}
+
+/**
+ * of_cdev_find
+ *
+ * @node: The node to find the cdev for, can be the device or a
+ *partition in the device
+ *
+ */
+struct cdev *of_cdev_find(struct device_node *node)
+{
+ return __of_cdev_find(node, NULL);
+}
+
+/**
+ * __of_find_path
+ *
+ * @node: The node to find the cdev for, can be the device or a
+ *partition in the device
+ * @part: Optionally, a description of a partition of @node. See of_find_path
+ * @outpath: if this function returns 0 outpath will contain the path belonging
+ * to the input path description. Must be freed with free().
+ * @flags: use OF_FIND_PATH_FLAGS_BB to return the .bb device if available
+ *
+ */
+static int __of_find_path(struct device_node *node, const char *part, char
**outpath, unsigned flags)
+{
+ bool add_bb = false;
+ struct cdev *cdev;
+
+ cdev = __of_cdev_find(node, part);
+ if (IS_ERR(cdev))
+ return PTR_ERR(cdev);
if ((flags & OF_FIND_PATH_FLAGS_BB) && cdev->mtd &&
mtd_can_have_bb(cdev->mtd))
diff --git a/include/of.h b/include/of.h
index c716f9283316..2b75ce63e185 100644
--- a/include/of.h
+++ b/include/of.h
@@ -331,6 +331,7 @@ int of_add_memory_bank(struct device_node *node, bool dump,
int r,
struct device *of_find_device_by_node_path(const char *path);
#define OF_FIND_PATH_FLAGS_BB 1/* return .bb device if
available */
int of_find_path(struct device_node *node, const char *propname, char
**outpath, unsigned flags);
+struct cdev *of_cdev_find(struct device_node *node);
int of_find_path_by_node(struct device_node *node, char **outpath, unsigned
flags);
struct device_node *of_find_node_by_devpath(struct device_node *root, const
char *path);
int of_register_fixup(int (*fixup)(struct device_node *, void *), void
*context);
--
2.39.2
[PATCH 07/18] commands: stat: remove code duplication for type info
stat prints a line with partitioning/type info for cdevs, but not all
cdevs have these, so we want to skip printing when it's empty.
Instead of duplicating the check, just utilize printf returning the
number of characters written.
Signed-off-by: Ahmad Fatoum
---
fs/fs.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index 368458cc54f8..ba60766a065a 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -69,6 +69,8 @@ EXPORT_SYMBOL(mkmodestr);
void cdev_print(const struct cdev *cdev)
{
+ int nbytes;
+
if (cdev->dev || cdev->master || cdev->partname) {
printf("Origin: %s", dev_name(cdev->dev) ?: "None");
if (cdev->master)
@@ -96,15 +98,17 @@ void cdev_print(const struct cdev *cdev)
}
printf("\n");
- if (cdev->filetype || cdev->dos_partition_type || *cdev->uuid) {
- if (cdev->filetype)
- printf("Filetype: %s\t",
file_type_to_string(cdev->filetype));
- if (cdev->dos_partition_type)
- printf("DOS parttype: 0x%02x\t",
cdev->dos_partition_type);
- if (*cdev->uuid)
- printf("UUID: %s", cdev->uuid);
+ nbytes = 0;
+
+ if (cdev->filetype)
+ nbytes += printf("Filetype: %s\t",
file_type_to_string(cdev->filetype));
+ if (cdev->dos_partition_type)
+ nbytes += printf("DOS parttype: 0x%02x\t",
cdev->dos_partition_type);
+ if (*cdev->uuid)
+ nbytes += printf("UUID: %s", cdev->uuid);
+
+ if (nbytes)
printf("\n");
- }
}
EXPORT_SYMBOL(cdev_print);
--
2.39.2
[PATCH 13/18] block: add cdev_is_block_(device,partition,disk) helpers
We look too much into struct cdev's guts. Let's add helpers to make
operating on block device cdevs more concise.
Signed-off-by: Ahmad Fatoum
---
include/block.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/block.h b/include/block.h
index 4dd2aa1f80ef..da258f509b41 100644
--- a/include/block.h
+++ b/include/block.h
@@ -58,4 +58,19 @@ static inline struct block_device
*cdev_get_block_device(const struct cdev *cdev
}
#endif
+static inline bool cdev_is_block_device(const struct cdev *cdev)
+{
+ return cdev_get_block_device(cdev) != NULL;
+}
+
+static inline bool cdev_is_block_partition(const struct cdev *cdev)
+{
+ return cdev_is_block_device(cdev) && cdev_is_partition(cdev);
+}
+
+static inline bool cdev_is_block_disk(const struct cdev *cdev)
+{
+ return cdev_is_block_device(cdev) && !cdev_is_partition(cdev);
+}
+
#endif /* __BLOCK_H */
--
2.39.2
[PATCH 03/18] cdev: fix for_each_cdev macro
The macro parameter 'c' was never used, instead hardcoding cdev. It worked so far anyway, because all users of for_each_cdev used cdev as the argument. Fix this. Signed-off-by: Ahmad Fatoum --- include/driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/driver.h b/include/driver.h index d33e0fcbccc9..e1ee3dc2dd7c 100644 --- a/include/driver.h +++ b/include/driver.h @@ -566,7 +566,7 @@ int cdev_truncate(struct cdev*, size_t size); loff_t cdev_unallocated_space(struct cdev *cdev); extern struct list_head cdev_list; -#define for_each_cdev(c) \ +#define for_each_cdev(cdev) \ list_for_each_entry(cdev, _list, list) #define DEVFS_PARTITION_FIXED (1U << 0) -- 2.39.2
[PATCH 09/18] cdev: record whether partition is parsed from OF
Later code will make it possible to define a on-disk-described partition
in the DT as well. For this reason, we can't assumed
DEVFS_PARTITION_FROM_TABLE to mean !DT, so let's add a dedicated flag
for that.
Signed-off-by: Ahmad Fatoum
---
drivers/of/partition.c | 5 +++--
fs/fs.c| 2 ++
include/driver.h | 5 +++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index a70e503cec9e..15943502ce17 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -74,6 +74,7 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct
device_node *node)
}
new->device_node = node;
+ new->flags |= DEVFS_PARTITION_FROM_OF;
if (IS_ENABLED(CONFIG_NVMEM) && of_device_is_compatible(node,
"nvmem-cells")) {
struct nvmem_device *nvmem = nvmem_partition_register(new);
@@ -162,7 +163,7 @@ int of_fixup_partitions(struct device_node *np, struct cdev
*cdev)
return 0;
list_for_each_entry(partcdev, >partitions, partition_entry) {
- if (partcdev->flags & DEVFS_PARTITION_FROM_TABLE)
+ if (!(partcdev->flags & DEVFS_PARTITION_FROM_OF))
continue;
n_parts++;
}
@@ -213,7 +214,7 @@ int of_fixup_partitions(struct device_node *np, struct cdev
*cdev)
u8 tmp[16 * 16]; /* Up to 64-bit address + 64-bit size */
loff_t partoffset;
- if (partcdev->flags & DEVFS_PARTITION_FROM_TABLE)
+ if (!(partcdev->flags & DEVFS_PARTITION_FROM_OF))
continue;
if (partcdev->mtd)
diff --git a/fs/fs.c b/fs/fs.c
index 1820e48393af..9d8aab268ca4 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -88,6 +88,8 @@ void cdev_print(const struct cdev *cdev)
printf(" fixed-partition");
if (cdev->flags & DEVFS_PARTITION_READONLY)
printf(" readonly-partition");
+ if (cdev->flags & DEVFS_PARTITION_FROM_OF)
+ printf(" of-partition");
if (cdev->flags & DEVFS_PARTITION_FROM_TABLE)
printf(" table-partition");
if (cdev->flags & DEVFS_IS_MCI_MAIN_PART_DEV)
diff --git a/include/driver.h b/include/driver.h
index 42e513a15603..118d2adb6750 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -584,8 +584,9 @@ extern struct list_head cdev_list;
#define DEVFS_PARTITION_FIXED (1U << 0)
#define DEVFS_PARTITION_READONLY (1U << 1)
#define DEVFS_IS_CHARACTER_DEV (1U << 3)
-#define DEVFS_PARTITION_FROM_TABLE (1U << 4)
-#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 5)
+#define DEVFS_IS_MCI_MAIN_PART_DEV (1U << 4)
+#define DEVFS_PARTITION_FROM_OF(1U << 5)
+#define DEVFS_PARTITION_FROM_TABLE (1U << 6)
struct cdev *devfs_add_partition(const char *devname, loff_t offset,
loff_t size, unsigned int flags, const char *name);
--
2.39.2
[PATCH 15/18] state: factor device path lookup into helper function
The #ifdef __BAREBOX__ is meant for easier synchronization with
dt-utils. We'll keep that intact, but move it out of the function to not
break reading flow. After sync, dt-utils would now need to implement
of_cdev_find
cdev_to_devpath
Signed-off-by: Ahmad Fatoum
---
common/state/state.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/common/state/state.c b/common/state/state.c
index 11cc86ff73be..88e246198fb8 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -581,6 +581,20 @@ void state_release(struct state *state)
free(state);
}
+#ifdef __BAREBOX__
+static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
+{
+ /*
+* We only accept partitions exactly mapping the barebox-state,
+* but dt-utils may need to set non-zero values here
+*/
+ *offset = 0;
+ *size = 0;
+
+ return basprintf("/dev/%s", cdev->name);
+}
+#endif
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -597,8 +611,9 @@ struct state *state_new_from_node(struct device_node *node,
bool readonly)
const char *alias;
uint32_t stridesize;
struct device_node *partition_node;
- off_t offset = 0;
- size_t size = 0;
+ struct cdev *cdev;
+ off_t offset;
+ size_t size;
alias = of_alias_get(node);
if (!alias) {
@@ -617,11 +632,8 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
-#ifdef __BAREBOX__
- ret = of_find_path_by_node(partition_node, >backend_path, 0);
-#else
- ret = of_get_devicepath(partition_node, >backend_path, ,
);
-#endif
+ cdev = of_cdev_find(partition_node);
+ ret = PTR_ERR_OR_ZERO(cdev);
if (ret) {
if (ret != -EPROBE_DEFER)
dev_err(>dev, "state failed to parse path to
backend: %s\n",
@@ -629,6 +641,10 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
+ state->backend_path = cdev_to_devpath(cdev, , );
+
+ pr_debug("%s: backend resolved to %s\n", node->full_name,
state->backend_path);
+
state->backend_reproducible_name =
of_get_reproducible_name(partition_node);
ret = of_property_read_string(node, "backend-type", _type);
--
2.39.2
[PATCH] net: phy: add driver for MotorComm PHY
From: Yegor Yefremov
Signed-off-by: Yegor Yefremov
---
drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile| 1 +
drivers/net/phy/motorcomm.c | 128
3 files changed, 134 insertions(+)
create mode 100644 drivers/net/phy/motorcomm.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd20e1de27..e95e2a3228 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -50,6 +50,11 @@ config MICREL_PHY
help
Supports the KSZ9021, VSC8201, KS8001 PHYs.
+config MOTORCOMM_PHY
+ bool "Driver for Motorcomm PHYs"
+ help
+ Currently supports the YT8511 PHY.
+
config NATIONAL_PHY
bool "Driver for National Semiconductor PHYs"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 83f46f11d3..26e4ad884d 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_LXT_PHY) += lxt.o
obj-$(CONFIG_MARVELL_PHY) += marvell.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
+obj-$(CONFIG_MOTORCOMM_PHY)+= motorcomm.o
obj-$(CONFIG_NATIONAL_PHY) += national.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
obj-$(CONFIG_SMSC_PHY) += smsc.o
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
new file mode 100644
index 00..4bcd84342c
--- /dev/null
+++ b/drivers/net/phy/motorcomm.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * drivers/net/phy/motorcomm.c
+ *
+ * Driver for Motorcomm PHYs
+ *
+ * Author: Peter Geis
+ */
+
+#include
+#include
+#include
+#include
+
+#define PHY_ID_YT8511 0x010a
+
+#define YT8511_PAGE_SELECT 0x1e
+#define YT8511_PAGE0x1f
+#define YT8511_EXT_CLK_GATE0x0c
+#define YT8511_EXT_DELAY_DRIVE 0x0d
+#define YT8511_EXT_SLEEP_CTRL 0x27
+
+/* 2b00 25m from pll
+ * 2b01 25m from xtl *default*
+ * 2b10 62.m from pll
+ * 2b11 125m from pll
+ */
+#define YT8511_CLK_125M(BIT(2) | BIT(1))
+#define YT8511_PLLON_SLP BIT(14)
+
+/* RX Delay enabled = 1.8ns 1000T, 8ns 10/100T */
+#define YT8511_DELAY_RXBIT(0)
+
+/* TX Gig-E Delay is bits 7:4, default 0x5
+ * TX Fast-E Delay is bits 15:12, default 0xf
+ * Delay = 150ps * N - 250ps
+ * On = 2000ps, off = 50ps
+ */
+#define YT8511_DELAY_GE_TX_EN (0xf << 4)
+#define YT8511_DELAY_GE_TX_DIS (0x2 << 4)
+#define YT8511_DELAY_FE_TX_EN (0xf << 12)
+#define YT8511_DELAY_FE_TX_DIS (0x2 << 12)
+
+static int yt8511_read_page(struct phy_device *phydev)
+{
+ return phy_read(phydev, YT8511_PAGE_SELECT);
+};
+
+static int yt8511_write_page(struct phy_device *phydev, int page)
+{
+ return phy_write(phydev, YT8511_PAGE_SELECT, page);
+};
+
+static int yt8511_config_init(struct phy_device *phydev)
+{
+ int oldpage, ret = 0;
+ unsigned int ge, fe;
+
+ oldpage = phy_select_page(phydev, YT8511_EXT_CLK_GATE);
+ if (oldpage < 0)
+ goto err_restore_page;
+
+ /* set rgmii delay mode */
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ ge = YT8511_DELAY_GE_TX_DIS;
+ fe = YT8511_DELAY_FE_TX_DIS;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_DIS;
+ fe = YT8511_DELAY_FE_TX_DIS;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ ge = YT8511_DELAY_GE_TX_EN;
+ fe = YT8511_DELAY_FE_TX_EN;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN;
+ fe = YT8511_DELAY_FE_TX_EN;
+ break;
+ default: /* do not support other modes */
+ ret = -EOPNOTSUPP;
+ goto err_restore_page;
+ }
+
+ ret = phy_modify(phydev, YT8511_PAGE, (YT8511_DELAY_RX |
YT8511_DELAY_GE_TX_EN), ge);
+ if (ret < 0)
+ goto err_restore_page;
+
+ /* set clock mode to 125mhz */
+ ret = phy_modify(phydev, YT8511_PAGE, 0, YT8511_CLK_125M);
+ if (ret < 0)
+ goto err_restore_page;
+
+ /* fast ethernet delay is in a separate page */
+ ret = phy_write(phydev, YT8511_PAGE_SELECT, YT8511_EXT_DELAY_DRIVE);
+ if (ret < 0)
+ goto err_restore_page;
+
+ ret = phy_modify(phydev, YT8511_PAGE, YT8511_DELAY_FE_TX_EN, fe);
+ if (ret < 0)
+ goto err_restore_page;
+
+ /* leave pll enabled in sleep */
+ ret = phy_write(phydev, YT8511_PAGE_SELECT, YT8511_EXT_SLEEP_CTRL);
+ if (ret < 0)
+ goto err_restore_page;
+
+ ret = phy_modify(phydev, YT8511_PAGE, 0, YT8511_PLLON_SLP);
+ if (ret < 0)
+ goto err_restore_page;
+
+err_restore_page:
+ return phy_restore_page(phydev, oldpage, ret);
+}
+
+static struct phy_driver
[PATCH 2/2] commands: stat: add basic handling for cdev links
cdev links are not symlinks and cdev_by_name will always resolve them.
As the barebox stat command is a convenience for VFS developers, it's
useful to have the command identify links, so let's teach it just that.
There's no behavior difference between specifying -L and not. This
should be rather achieved by removing the concept of cdev links and
using symlinks instead, but that's some refactoring for another time.
Signed-off-by: Ahmad Fatoum
---
fs/devfs-core.c | 3 +++
fs/fs.c | 14 +++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index fbcf68e81597..68a41ed20dd1 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -49,6 +49,9 @@ int devfs_partition_complete(struct string_list *sl, char
*instr)
struct cdev *cdev_readlink(struct cdev *cdev)
{
+ if (!cdev)
+ return NULL;
+
if (cdev->link)
cdev = cdev->link;
diff --git a/fs/fs.c b/fs/fs.c
index 311571ba3088..68e6bf5735f0 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -115,7 +115,8 @@ void stat_print(const char *filename, const struct stat *st)
struct block_device *bdev = NULL;
struct fs_device *fdev;
struct cdev *cdev = NULL;
- const char *type = NULL;
+ const char *type = NULL, *typeprefix = "";
+ bool is_cdev_link = false;
char modestr[11];
mkmodestr(st->st_mode, modestr);
@@ -136,8 +137,12 @@ void stat_print(const char *filename, const struct stat
*st)
path = canonicalize_path(filename);
if (path) {
const char *devicefile = devpath_to_name(path);
+ struct cdev *lcdev;
- cdev = cdev_by_name(devicefile);
+ lcdev = lcdev_by_name(devicefile);
+ cdev = cdev_readlink(lcdev);
+ if (cdev != lcdev)
+ is_cdev_link = true;
if (cdev)
bdev = cdev_get_block_device(cdev);
@@ -156,6 +161,9 @@ void stat_print(const char *filename, const struct stat *st)
printf(" -> ", ERR_PTR(ret));
else
printf(" -> %s", realname);
+ } else if (is_cdev_link) {
+ printf(" ~> %s", cdev->name);
+ typeprefix = "cdev link to ";
}
printf("\n");
@@ -166,7 +174,7 @@ void stat_print(const char *filename, const struct stat *st)
(u64)bdev->num_blocks, 1 << bdev->blockbits);
if (type)
- printf(" %s", type);
+ printf(" %s%s", typeprefix, type);
fdev = get_fsdevice_by_path(filename);
--
2.39.2
[PATCH 1/2] commands: stat: print symlink destination when called without -L
This aligns behavior with usual implementations of stat.
Signed-off-by: Ahmad Fatoum
---
fs/fs.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index 368458cc54f8..311571ba3088 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -130,8 +130,6 @@ void stat_print(const char *filename, const struct stat *st)
case S_IFREG:type = "regular file"; break;
}
- printf(" File: %s\n", filename);
-
if (st->st_mode & S_IFCHR) {
char *path;
@@ -147,6 +145,21 @@ void stat_print(const char *filename, const struct stat
*st)
}
}
+ printf(" File: %s", filename);
+
+ if (S_ISLNK(st->st_mode)) {
+ char realname[PATH_MAX] = {};
+ int ret;
+
+ ret = readlink(filename, realname, PATH_MAX - 1);
+ if (ret)
+ printf(" -> ", ERR_PTR(ret));
+ else
+ printf(" -> %s", realname);
+ }
+
+ printf("\n");
+
printf(" Size: %-20llu", st->st_size);
if (bdev)
printf("Blocks: %llu\tIO Block: %u\t",
--
2.39.2
Re: [PATCH] Revert "dma: use dma/cpu conversions correctly in dma_map/unmap_single"
Hello Denis,
On 31.05.23 15:02, Denis Orlov wrote:
> Hi!
>
>>
>> On 31.05.23 12:23, Sascha Hauer wrote:
>>> This reverts commit d13d870986eeecc58d8652268557e4a159b9d4c4.
>>>
>>> While the patch itself is correct, it at least breaks USB on the
>>> Raspberry Pi 3b.
>>>
>>> With this patch dma_sync_single_for_device() is passed the DMA address.
>>> This is correct as even the prototype suggests that it should get a
>>> dma_addr_t. Unfortunately this is not what the function implements and
>>> also not what the users expect. Most if not all users simply pass a CPU
>>> pointer casted to unsigned long. dma_sync_single_for_device() on ARM
>>> then takes the DMA address as a CPU pointer and does cache maintenance
>>> on it.
>>>
>>> Before we can merge this patch again dma_sync_single_for_device() must
>>> get a struct device * argument and (on ARM) the cpu_to_dma() conversion
>>> must be reverted before doing cache maintenance.
>>
>> @Denis, could you give some background on your patch? I assume this was
>> for MIPS? Did this patch fix breakage for you? In what driver? Maybe
>> a follow-up patch that fixes your particular breakage while not breaking
>> ARM could be found until that wart is cleaned up for good.
>
> I'm okay with this patch being reverted, sorry for any inconvenience.
> Will try to come up with a better one in the meantime.
>
> It appears that MIPS was *always* somewhat broken in this regard.
> Without this patch, we end up calling dma_sync_single_for_device()
> with a virtual address, and dma_sync_single_for_cpu() with a physical
> one. As on MIPS phys/virt addresses do not map 1:1 to each other, we
> can't really do anything sensible on the MIPS side in this case. Either
> map or unmap calls will be broken. On actual boards this will result in
> address errors with any driver that does DMA mappings.
>
> I originally sent an RFC with the whole streaming DMA interface rework,
> but I was a bit hesitant if such changes are actually needed.
I had forgotten about that one. Approach looks fine IMO. Feel free to rebase
and resend. I can give this a test on a number of ARM boards I have in the
remote lab here.
> It occured
> to me that they are useful in theory, however, nothing in barebox seemed
> to require it at the moment. So I just resorted to smaller changes that
> were enough to fix MIPS, but I must have totally forgotten to check if
> other archs are fine with those changes applied.
>
> I don't like having to do cpu_to_dma() in common code just to call
> dma_to_cpu() on the arch side. But it seems like we have to do it in the
> most generic case.
Ye, it sounds wrong, but it feels right?!
Cheers,
Ahmad
>
>>
>> Cheers,
>> Ahmad
>>
>>
>>> ---
>>> drivers/dma/map.c | 10 ++
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/dma/map.c b/drivers/dma/map.c
>>> index fea04c38a3..114c0f7db3 100644
>>> --- a/drivers/dma/map.c
>>> +++ b/drivers/dma/map.c
>>> @@ -23,15 +23,17 @@ static inline void *dma_to_cpu(struct device *dev,
>>> dma_addr_t addr)
>>> dma_addr_t dma_map_single(struct device *dev, void *ptr, size_t size,
>>> enum dma_data_direction dir)
>>> {
>>> - dma_addr_t ret = cpu_to_dma(dev, ptr);
>>> + unsigned long addr = (unsigned long)ptr;
>>>
>>> - dma_sync_single_for_device(ret, size, dir);
>>> + dma_sync_single_for_device(addr, size, dir);
>>>
>>> - return ret;
>>> + return cpu_to_dma(dev, ptr);
>>> }
>>>
>>> void dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
>>> enum dma_data_direction dir)
>>> {
>>> - dma_sync_single_for_cpu(dma_addr, size, dir);
>>> + unsigned long addr = (unsigned long)dma_to_cpu(dev, dma_addr);
>>> +
>>> + dma_sync_single_for_cpu(addr, size, dir);
>>> }
>>
>> --
>> Pengutronix e.K. | |
>> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH] Revert "dma: use dma/cpu conversions correctly in dma_map/unmap_single"
Hi!
>
> On 31.05.23 12:23, Sascha Hauer wrote:
> > This reverts commit d13d870986eeecc58d8652268557e4a159b9d4c4.
> >
> > While the patch itself is correct, it at least breaks USB on the
> > Raspberry Pi 3b.
> >
> > With this patch dma_sync_single_for_device() is passed the DMA address.
> > This is correct as even the prototype suggests that it should get a
> > dma_addr_t. Unfortunately this is not what the function implements and
> > also not what the users expect. Most if not all users simply pass a CPU
> > pointer casted to unsigned long. dma_sync_single_for_device() on ARM
> > then takes the DMA address as a CPU pointer and does cache maintenance
> > on it.
> >
> > Before we can merge this patch again dma_sync_single_for_device() must
> > get a struct device * argument and (on ARM) the cpu_to_dma() conversion
> > must be reverted before doing cache maintenance.
>
> @Denis, could you give some background on your patch? I assume this was
> for MIPS? Did this patch fix breakage for you? In what driver? Maybe
> a follow-up patch that fixes your particular breakage while not breaking
> ARM could be found until that wart is cleaned up for good.
I'm okay with this patch being reverted, sorry for any inconvenience.
Will try to come up with a better one in the meantime.
It appears that MIPS was *always* somewhat broken in this regard.
Without this patch, we end up calling dma_sync_single_for_device()
with a virtual address, and dma_sync_single_for_cpu() with a physical
one. As on MIPS phys/virt addresses do not map 1:1 to each other, we
can't really do anything sensible on the MIPS side in this case. Either
map or unmap calls will be broken. On actual boards this will result in
address errors with any driver that does DMA mappings.
I originally sent an RFC with the whole streaming DMA interface rework,
but I was a bit hesitant if such changes are actually needed. It occured
to me that they are useful in theory, however, nothing in barebox seemed
to require it at the moment. So I just resorted to smaller changes that
were enough to fix MIPS, but I must have totally forgotten to check if
other archs are fine with those changes applied.
I don't like having to do cpu_to_dma() in common code just to call
dma_to_cpu() on the arch side. But it seems like we have to do it in the
most generic case.
>
> Cheers,
> Ahmad
>
>
> > ---
> > drivers/dma/map.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/map.c b/drivers/dma/map.c
> > index fea04c38a3..114c0f7db3 100644
> > --- a/drivers/dma/map.c
> > +++ b/drivers/dma/map.c
> > @@ -23,15 +23,17 @@ static inline void *dma_to_cpu(struct device *dev,
> > dma_addr_t addr)
> > dma_addr_t dma_map_single(struct device *dev, void *ptr, size_t size,
> > enum dma_data_direction dir)
> > {
> > - dma_addr_t ret = cpu_to_dma(dev, ptr);
> > + unsigned long addr = (unsigned long)ptr;
> >
> > - dma_sync_single_for_device(ret, size, dir);
> > + dma_sync_single_for_device(addr, size, dir);
> >
> > - return ret;
> > + return cpu_to_dma(dev, ptr);
> > }
> >
> > void dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
> > enum dma_data_direction dir)
> > {
> > - dma_sync_single_for_cpu(dma_addr, size, dir);
> > + unsigned long addr = (unsigned long)dma_to_cpu(dev, dma_addr);
> > +
> > + dma_sync_single_for_cpu(addr, size, dir);
> > }
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
>
Re: [PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
On 31.05.23 14:38, Sascha Hauer wrote:
> On Wed, May 31, 2023 at 01:58:33PM +0200, Ahmad Fatoum wrote:
>>> From b6e5c92682467496bd9c57918996f1feffda2dd6 Mon Sep 17 00:00:00 2001
>>> From: Sascha Hauer
>>> Date: Wed, 31 May 2023 11:58:51 +0200
>>> Subject: [PATCH] ARM: mmu_32: fix setting up zero page when it is in SDRAM
>>>
>>> We used to skip setting the zero page to faulting when SDRAM starts at
>>> 0x0. As bootm code now explicitly sets the zero page accessible before
>>> copying ATAGs there this should no longer be necessary, so
>>> unconditionally set the zero page to faulting during MMU startup. This
>>> also moves the zero page and vector table setup after the point the
>>> SDRAM has been mapped cachable, because otherwise the zero page and
>>> possibly the vector table mapping would be overwritten.
>>>
>>> Signed-off-by: Sascha Hauer
>>> ---
>>> arch/arm/cpu/mmu_32.c | 23 +--
>>> 1 file changed, 9 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
>>> index c4e5a3bb0a..14775768a3 100644
>>> --- a/arch/arm/cpu/mmu_32.c
>>> +++ b/arch/arm/cpu/mmu_32.c
>>> @@ -461,19 +461,14 @@ static int set_vector_table(unsigned long adr)
>>>
>>> static void create_zero_page(void)
>>
>> Is this commit incomplete? Vectors should be set up unconditionally and
>> create_zero_page should be called after it.
>
> Vectors are set up unconditionally and create_zero_page() is called the
> same way as before.
So zero page is requested first and then on platforms with vector at
address 0 requesting fails and we are left without configured IVT?
>
> Sascha
>
>>
>>> {
>>> - struct resource *zero_sdram;
>>> + /*
>>> +* In case the zero page is in SDRAM request it to prevent others
>>> +* from using it
>>> +*/
>>> + request_sdram_region("zero page", 0x0, PAGE_SIZE);
>>>
>>> - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
>>> - if (zero_sdram) {
>>> - /*
>>> -* Here we would need to set the second level page table
>>> -* entry to faulting. This is not yet implemented.
>>> -*/
>>> - pr_debug("zero page is in SDRAM area, currently not
>>> supported\n");
>>> - } else {
>>> - zero_page_faulting();
>>> - pr_debug("Created zero page\n");
>>> - }
>>> + zero_page_faulting();
>>> + pr_debug("Created zero page\n");
>>> }
>>>
>>> /*
>>> @@ -530,8 +525,6 @@ void __mmu_init(bool mmu_on)
>>>
>>> pr_debug("ttb: 0x%p\n", ttb);
>>>
>>> - vectors_init();
>>> -
>>> /*
>>> * Early mmu init will have mapped everything but the initial memory
>>> area
>>> * (excluding final OPTEE_SIZE bytes) uncached. We have now discovered
>>> @@ -552,6 +545,8 @@ void __mmu_init(bool mmu_on)
>>>
>>> remap_range((void *)pos, bank->start + bank->size - pos,
>>> MAP_CACHED);
>>> }
>>> +
>>> + vectors_init();
>>> }
>>>
>>> /*
>>
>> --
>> Pengutronix e.K. | |
>> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
On Wed, May 31, 2023 at 01:58:33PM +0200, Ahmad Fatoum wrote:
> > From b6e5c92682467496bd9c57918996f1feffda2dd6 Mon Sep 17 00:00:00 2001
> > From: Sascha Hauer
> > Date: Wed, 31 May 2023 11:58:51 +0200
> > Subject: [PATCH] ARM: mmu_32: fix setting up zero page when it is in SDRAM
> >
> > We used to skip setting the zero page to faulting when SDRAM starts at
> > 0x0. As bootm code now explicitly sets the zero page accessible before
> > copying ATAGs there this should no longer be necessary, so
> > unconditionally set the zero page to faulting during MMU startup. This
> > also moves the zero page and vector table setup after the point the
> > SDRAM has been mapped cachable, because otherwise the zero page and
> > possibly the vector table mapping would be overwritten.
> >
> > Signed-off-by: Sascha Hauer
> > ---
> > arch/arm/cpu/mmu_32.c | 23 +--
> > 1 file changed, 9 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
> > index c4e5a3bb0a..14775768a3 100644
> > --- a/arch/arm/cpu/mmu_32.c
> > +++ b/arch/arm/cpu/mmu_32.c
> > @@ -461,19 +461,14 @@ static int set_vector_table(unsigned long adr)
> >
> > static void create_zero_page(void)
>
> Is this commit incomplete? Vectors should be set up unconditionally and
> create_zero_page should be called after it.
Vectors are set up unconditionally and create_zero_page() is called the
same way as before.
Sascha
>
> > {
> > - struct resource *zero_sdram;
> > + /*
> > +* In case the zero page is in SDRAM request it to prevent others
> > +* from using it
> > +*/
> > + request_sdram_region("zero page", 0x0, PAGE_SIZE);
> >
> > - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
> > - if (zero_sdram) {
> > - /*
> > -* Here we would need to set the second level page table
> > -* entry to faulting. This is not yet implemented.
> > -*/
> > - pr_debug("zero page is in SDRAM area, currently not
> > supported\n");
> > - } else {
> > - zero_page_faulting();
> > - pr_debug("Created zero page\n");
> > - }
> > + zero_page_faulting();
> > + pr_debug("Created zero page\n");
> > }
> >
> > /*
> > @@ -530,8 +525,6 @@ void __mmu_init(bool mmu_on)
> >
> > pr_debug("ttb: 0x%p\n", ttb);
> >
> > - vectors_init();
> > -
> > /*
> > * Early mmu init will have mapped everything but the initial memory
> > area
> > * (excluding final OPTEE_SIZE bytes) uncached. We have now discovered
> > @@ -552,6 +545,8 @@ void __mmu_init(bool mmu_on)
> >
> > remap_range((void *)pos, bank->start + bank->size - pos,
> > MAP_CACHED);
> > }
> > +
> > + vectors_init();
> > }
> >
> > /*
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
Hello Sascha,
On 31.05.23 13:21, Sascha Hauer wrote:
> On Wed, May 31, 2023 at 12:45:17PM +0200, Ahmad Fatoum wrote:
>> On 31.05.23 12:35, Sascha Hauer wrote:
>>> We used to skip setting the zero page to faulting when SDRAM starts
>>> at 0x0. As bootm code now explicitly sets the zero page accessible
>>> before copying ATAGs there this should no longer be necessary, so
>>> unconditionally set the zero page to faulting during MMU startup.
>>> This also moves the zero page setup after the point the SDRAM has
>>> been mapped cachable, because otherwise the zero page setup would
>>> be overwritten.
>>>
>>> Signed-off-by: Sascha Hauer
>>> ---
>>> arch/arm/cpu/mmu_32.c | 26 +++---
>>> 1 file changed, 7 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
>>> index c4e5a3bb0a..fdbc0293a3 100644
>>> --- a/arch/arm/cpu/mmu_32.c
>>> +++ b/arch/arm/cpu/mmu_32.c
>>> @@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
>>> return -EINVAL;
>>> }
>>>
>>> -static void create_zero_page(void)
>>> -{
>>> - struct resource *zero_sdram;
>>> -
>>> - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
>>> - if (zero_sdram) {
>>> - /*
>>> -* Here we would need to set the second level page table
>>> -* entry to faulting. This is not yet implemented.
>>> -*/
>>> - pr_debug("zero page is in SDRAM area, currently not
>>> supported\n");
>>> - } else {
>>> - zero_page_faulting();
>>> - pr_debug("Created zero page\n");
>>> - }
>>> -}
>>> -
>>> /*
>>> * Map vectors and zero page
>>> */
>>> @@ -487,7 +470,6 @@ static void vectors_init(void)
>>> */
>>> if (!set_vector_table((unsigned long)__exceptions_start)) {
>>> arm_fixup_vectors();
>>> - create_zero_page();
>>> return;
>>> }
>>>
>>> @@ -495,7 +477,6 @@ static void vectors_init(void)
>>> * Next try high vectors at 0x.
>>> */
>>> if (!set_vector_table(ARM_HIGH_VECTORS)) {
>>> - create_zero_page();
>>> create_vector_table(ARM_HIGH_VECTORS);
>>> return;
>>> }
>>> @@ -552,6 +533,13 @@ void __mmu_init(bool mmu_on)
>>>
>>> remap_range((void *)pos, bank->start + bank->size - pos,
>>> MAP_CACHED);
>>> }
>>> +
>>> + /*
>>> +* In case the zero page is in SDRAM request it to prevent others
>>> +* from using it
>>> +*/
>>> + request_sdram_region("zero page", 0x0, PAGE_SIZE);
>>> + zero_page_faulting();
>>
>> I think this would break the case of having low vectors (at address 0).
>> We have vector_table requested if that's the case, so we need to check:
>>
>> if (!zero_page_in_sdram() || !zero_page_already_sdram_requested())
>> zero_page_faulting();
>
> You are right. How about this one?
>
> --8<--
>
>
> From b6e5c92682467496bd9c57918996f1feffda2dd6 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer
> Date: Wed, 31 May 2023 11:58:51 +0200
> Subject: [PATCH] ARM: mmu_32: fix setting up zero page when it is in SDRAM
>
> We used to skip setting the zero page to faulting when SDRAM starts at
> 0x0. As bootm code now explicitly sets the zero page accessible before
> copying ATAGs there this should no longer be necessary, so
> unconditionally set the zero page to faulting during MMU startup. This
> also moves the zero page and vector table setup after the point the
> SDRAM has been mapped cachable, because otherwise the zero page and
> possibly the vector table mapping would be overwritten.
>
> Signed-off-by: Sascha Hauer
> ---
> arch/arm/cpu/mmu_32.c | 23 +--
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
> index c4e5a3bb0a..14775768a3 100644
> --- a/arch/arm/cpu/mmu_32.c
> +++ b/arch/arm/cpu/mmu_32.c
> @@ -461,19 +461,14 @@ static int set_vector_table(unsigned long adr)
>
> static void create_zero_page(void)
Is this commit incomplete? Vectors should be set up unconditionally and
create_zero_page should be called after it.
> {
> - struct resource *zero_sdram;
> + /*
> + * In case the zero page is in SDRAM request it to prevent others
> + * from using it
> + */
> + request_sdram_region("zero page", 0x0, PAGE_SIZE);
>
> - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
> - if (zero_sdram) {
> - /*
> - * Here we would need to set the second level page table
> - * entry to faulting. This is not yet implemented.
> - */
> - pr_debug("zero page is in SDRAM area, currently not
> supported\n");
> - } else {
> - zero_page_faulting();
> - pr_debug("Created zero page\n");
> - }
> + zero_page_faulting();
> + pr_debug("Created zero page\n");
> }
>
> /*
> @@ -530,8
Re: [PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
On Wed, May 31, 2023 at 12:45:17PM +0200, Ahmad Fatoum wrote:
> On 31.05.23 12:35, Sascha Hauer wrote:
> > We used to skip setting the zero page to faulting when SDRAM starts
> > at 0x0. As bootm code now explicitly sets the zero page accessible
> > before copying ATAGs there this should no longer be necessary, so
> > unconditionally set the zero page to faulting during MMU startup.
> > This also moves the zero page setup after the point the SDRAM has
> > been mapped cachable, because otherwise the zero page setup would
> > be overwritten.
> >
> > Signed-off-by: Sascha Hauer
> > ---
> > arch/arm/cpu/mmu_32.c | 26 +++---
> > 1 file changed, 7 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
> > index c4e5a3bb0a..fdbc0293a3 100644
> > --- a/arch/arm/cpu/mmu_32.c
> > +++ b/arch/arm/cpu/mmu_32.c
> > @@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
> > return -EINVAL;
> > }
> >
> > -static void create_zero_page(void)
> > -{
> > - struct resource *zero_sdram;
> > -
> > - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
> > - if (zero_sdram) {
> > - /*
> > -* Here we would need to set the second level page table
> > -* entry to faulting. This is not yet implemented.
> > -*/
> > - pr_debug("zero page is in SDRAM area, currently not
> > supported\n");
> > - } else {
> > - zero_page_faulting();
> > - pr_debug("Created zero page\n");
> > - }
> > -}
> > -
> > /*
> > * Map vectors and zero page
> > */
> > @@ -487,7 +470,6 @@ static void vectors_init(void)
> > */
> > if (!set_vector_table((unsigned long)__exceptions_start)) {
> > arm_fixup_vectors();
> > - create_zero_page();
> > return;
> > }
> >
> > @@ -495,7 +477,6 @@ static void vectors_init(void)
> > * Next try high vectors at 0x.
> > */
> > if (!set_vector_table(ARM_HIGH_VECTORS)) {
> > - create_zero_page();
> > create_vector_table(ARM_HIGH_VECTORS);
> > return;
> > }
> > @@ -552,6 +533,13 @@ void __mmu_init(bool mmu_on)
> >
> > remap_range((void *)pos, bank->start + bank->size - pos,
> > MAP_CACHED);
> > }
> > +
> > + /*
> > +* In case the zero page is in SDRAM request it to prevent others
> > +* from using it
> > +*/
> > + request_sdram_region("zero page", 0x0, PAGE_SIZE);
> > + zero_page_faulting();
>
> I think this would break the case of having low vectors (at address 0).
> We have vector_table requested if that's the case, so we need to check:
>
> if (!zero_page_in_sdram() || !zero_page_already_sdram_requested())
> zero_page_faulting();
You are right. How about this one?
--8<--
>From b6e5c92682467496bd9c57918996f1feffda2dd6 Mon Sep 17 00:00:00 2001
From: Sascha Hauer
Date: Wed, 31 May 2023 11:58:51 +0200
Subject: [PATCH] ARM: mmu_32: fix setting up zero page when it is in SDRAM
We used to skip setting the zero page to faulting when SDRAM starts at
0x0. As bootm code now explicitly sets the zero page accessible before
copying ATAGs there this should no longer be necessary, so
unconditionally set the zero page to faulting during MMU startup. This
also moves the zero page and vector table setup after the point the
SDRAM has been mapped cachable, because otherwise the zero page and
possibly the vector table mapping would be overwritten.
Signed-off-by: Sascha Hauer
---
arch/arm/cpu/mmu_32.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
index c4e5a3bb0a..14775768a3 100644
--- a/arch/arm/cpu/mmu_32.c
+++ b/arch/arm/cpu/mmu_32.c
@@ -461,19 +461,14 @@ static int set_vector_table(unsigned long adr)
static void create_zero_page(void)
{
- struct resource *zero_sdram;
+ /*
+* In case the zero page is in SDRAM request it to prevent others
+* from using it
+*/
+ request_sdram_region("zero page", 0x0, PAGE_SIZE);
- zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
- if (zero_sdram) {
- /*
-* Here we would need to set the second level page table
-* entry to faulting. This is not yet implemented.
-*/
- pr_debug("zero page is in SDRAM area, currently not
supported\n");
- } else {
- zero_page_faulting();
- pr_debug("Created zero page\n");
- }
+ zero_page_faulting();
+ pr_debug("Created zero page\n");
}
/*
@@ -530,8 +525,6 @@ void __mmu_init(bool mmu_on)
pr_debug("ttb: 0x%p\n", ttb);
- vectors_init();
-
/*
* Early mmu init will have mapped everything but the initial memory
area
* (excluding final OPTEE_SIZE bytes)
Re: [PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
On 31.05.23 12:35, Sascha Hauer wrote:
> We used to skip setting the zero page to faulting when SDRAM starts
> at 0x0. As bootm code now explicitly sets the zero page accessible
> before copying ATAGs there this should no longer be necessary, so
> unconditionally set the zero page to faulting during MMU startup.
> This also moves the zero page setup after the point the SDRAM has
> been mapped cachable, because otherwise the zero page setup would
> be overwritten.
>
> Signed-off-by: Sascha Hauer
> ---
> arch/arm/cpu/mmu_32.c | 26 +++---
> 1 file changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
> index c4e5a3bb0a..fdbc0293a3 100644
> --- a/arch/arm/cpu/mmu_32.c
> +++ b/arch/arm/cpu/mmu_32.c
> @@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
> return -EINVAL;
> }
>
> -static void create_zero_page(void)
> -{
> - struct resource *zero_sdram;
> -
> - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
> - if (zero_sdram) {
> - /*
> - * Here we would need to set the second level page table
> - * entry to faulting. This is not yet implemented.
> - */
> - pr_debug("zero page is in SDRAM area, currently not
> supported\n");
> - } else {
> - zero_page_faulting();
> - pr_debug("Created zero page\n");
> - }
> -}
> -
> /*
> * Map vectors and zero page
> */
> @@ -487,7 +470,6 @@ static void vectors_init(void)
>*/
> if (!set_vector_table((unsigned long)__exceptions_start)) {
> arm_fixup_vectors();
> - create_zero_page();
> return;
> }
>
> @@ -495,7 +477,6 @@ static void vectors_init(void)
>* Next try high vectors at 0x.
>*/
> if (!set_vector_table(ARM_HIGH_VECTORS)) {
> - create_zero_page();
> create_vector_table(ARM_HIGH_VECTORS);
> return;
> }
> @@ -552,6 +533,13 @@ void __mmu_init(bool mmu_on)
>
> remap_range((void *)pos, bank->start + bank->size - pos,
> MAP_CACHED);
> }
> +
> + /*
> + * In case the zero page is in SDRAM request it to prevent others
> + * from using it
> + */
> + request_sdram_region("zero page", 0x0, PAGE_SIZE);
> + zero_page_faulting();
I think this would break the case of having low vectors (at address 0).
We have vector_table requested if that's the case, so we need to check:
if (!zero_page_in_sdram() || !zero_page_already_sdram_requested())
zero_page_faulting();
Cheers,
Ahmad
> }
>
> /*
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] ARM: set zero page accessible before copying ATAGs there
On 31.05.23 12:35, Sascha Hauer wrote:
> We used skip setting up the zero page as faulting when the SDRAM
> starts at 0x0. One reason for doing that was that ATAGs will be copied
> there in that case. Call zero_page_access() if necessary to be able
> to set the zero page to faulting during barebox startup in the next
> step.
>
> Signed-off-by: Sascha Hauer
Reviewed-by: Ahmad Fatoum
> ---
> arch/arm/lib32/armlinux.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> index 6cb7d4b5f3..eb30f4a952 100644
> --- a/arch/arm/lib32/armlinux.c
> +++ b/arch/arm/lib32/armlinux.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -265,8 +266,12 @@ void start_linux(void *adr, int swap, unsigned long
> initrd_address,
> pr_debug("booting kernel with devicetree\n");
> params = oftree;
> } else {
> - setup_tags(initrd_address, initrd_size, swap);
> params = armlinux_get_bootparams();
> +
> + if ((unsigned long)params < PAGE_SIZE)
> + zero_page_access();
> +
> + setup_tags(initrd_address, initrd_size, swap);
> }
> architecture = armlinux_get_architecture();
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v2 1/2] scripts: omap3-usb-loader: fix heap overflow
Gna, forget this. Wrong patches sent. Sascha On Wed, May 31, 2023 at 12:26:08PM +0200, Sascha Hauer wrote: > From: Ahmad Fatoum > > Newer GCC versions correctly warn that the buffer allocated by realloc > is too small. Correct the size. > > Signed-off-by: Ahmad Fatoum > Link: https://lore.barebox.org/20230531062703.670521-3-ah...@a3f.at > Signed-off-by: Sascha Hauer > --- > scripts/omap3-usb-loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/omap3-usb-loader.c b/scripts/omap3-usb-loader.c > index a8d626c32f..31a03be8e7 100644 > --- a/scripts/omap3-usb-loader.c > +++ b/scripts/omap3-usb-loader.c > @@ -784,7 +784,7 @@ int main(int argc, char *argv[]) > file.addr = OMAP_BASE_ADDRESS; > > /* commit the file object with the processor > specified base address */ > - args->files = realloc(args->files, filecount); > + args->files = realloc(args->files, filecount * > sizeof(*args->files)); > args->numfiles = filecount; > args->files[filecount - 1] = malloc(sizeof > (file)); > memcpy(args->files[filecount - 1], , > sizeof (file)); > -- > 2.39.2 > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH v2 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
We used to skip setting the zero page to faulting when SDRAM starts
at 0x0. As bootm code now explicitly sets the zero page accessible
before copying ATAGs there this should no longer be necessary, so
unconditionally set the zero page to faulting during MMU startup.
This also moves the zero page setup after the point the SDRAM has
been mapped cachable, because otherwise the zero page setup would
be overwritten.
Signed-off-by: Sascha Hauer
---
arch/arm/cpu/mmu_32.c | 26 +++---
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
index c4e5a3bb0a..fdbc0293a3 100644
--- a/arch/arm/cpu/mmu_32.c
+++ b/arch/arm/cpu/mmu_32.c
@@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
return -EINVAL;
}
-static void create_zero_page(void)
-{
- struct resource *zero_sdram;
-
- zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
- if (zero_sdram) {
- /*
-* Here we would need to set the second level page table
-* entry to faulting. This is not yet implemented.
-*/
- pr_debug("zero page is in SDRAM area, currently not
supported\n");
- } else {
- zero_page_faulting();
- pr_debug("Created zero page\n");
- }
-}
-
/*
* Map vectors and zero page
*/
@@ -487,7 +470,6 @@ static void vectors_init(void)
*/
if (!set_vector_table((unsigned long)__exceptions_start)) {
arm_fixup_vectors();
- create_zero_page();
return;
}
@@ -495,7 +477,6 @@ static void vectors_init(void)
* Next try high vectors at 0x.
*/
if (!set_vector_table(ARM_HIGH_VECTORS)) {
- create_zero_page();
create_vector_table(ARM_HIGH_VECTORS);
return;
}
@@ -552,6 +533,13 @@ void __mmu_init(bool mmu_on)
remap_range((void *)pos, bank->start + bank->size - pos,
MAP_CACHED);
}
+
+ /*
+* In case the zero page is in SDRAM request it to prevent others
+* from using it
+*/
+ request_sdram_region("zero page", 0x0, PAGE_SIZE);
+ zero_page_faulting();
}
/*
--
2.39.2
[PATCH v2 1/2] ARM: set zero page accessible before copying ATAGs there
We used skip setting up the zero page as faulting when the SDRAM
starts at 0x0. One reason for doing that was that ATAGs will be copied
there in that case. Call zero_page_access() if necessary to be able
to set the zero page to faulting during barebox startup in the next
step.
Signed-off-by: Sascha Hauer
---
arch/arm/lib32/armlinux.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
index 6cb7d4b5f3..eb30f4a952 100644
--- a/arch/arm/lib32/armlinux.c
+++ b/arch/arm/lib32/armlinux.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -265,8 +266,12 @@ void start_linux(void *adr, int swap, unsigned long
initrd_address,
pr_debug("booting kernel with devicetree\n");
params = oftree;
} else {
- setup_tags(initrd_address, initrd_size, swap);
params = armlinux_get_bootparams();
+
+ if ((unsigned long)params < PAGE_SIZE)
+ zero_page_access();
+
+ setup_tags(initrd_address, initrd_size, swap);
}
architecture = armlinux_get_architecture();
--
2.39.2
Re: [PATCH] Revert "dma: use dma/cpu conversions correctly in dma_map/unmap_single"
On 31.05.23 12:23, Sascha Hauer wrote:
> This reverts commit d13d870986eeecc58d8652268557e4a159b9d4c4.
>
> While the patch itself is correct, it at least breaks USB on the
> Raspberry Pi 3b.
>
> With this patch dma_sync_single_for_device() is passed the DMA address.
> This is correct as even the prototype suggests that it should get a
> dma_addr_t. Unfortunately this is not what the function implements and
> also not what the users expect. Most if not all users simply pass a CPU
> pointer casted to unsigned long. dma_sync_single_for_device() on ARM
> then takes the DMA address as a CPU pointer and does cache maintenance
> on it.
>
> Before we can merge this patch again dma_sync_single_for_device() must
> get a struct device * argument and (on ARM) the cpu_to_dma() conversion
> must be reverted before doing cache maintenance.
@Denis, could you give some background on your patch? I assume this was
for MIPS? Did this patch fix breakage for you? In what driver? Maybe
a follow-up patch that fixes your particular breakage while not breaking
ARM could be found until that wart is cleaned up for good.
Cheers,
Ahmad
> ---
> drivers/dma/map.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/map.c b/drivers/dma/map.c
> index fea04c38a3..114c0f7db3 100644
> --- a/drivers/dma/map.c
> +++ b/drivers/dma/map.c
> @@ -23,15 +23,17 @@ static inline void *dma_to_cpu(struct device *dev,
> dma_addr_t addr)
> dma_addr_t dma_map_single(struct device *dev, void *ptr, size_t size,
> enum dma_data_direction dir)
> {
> - dma_addr_t ret = cpu_to_dma(dev, ptr);
> + unsigned long addr = (unsigned long)ptr;
>
> - dma_sync_single_for_device(ret, size, dir);
> + dma_sync_single_for_device(addr, size, dir);
>
> - return ret;
> + return cpu_to_dma(dev, ptr);
> }
>
> void dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
> enum dma_data_direction dir)
> {
> - dma_sync_single_for_cpu(dma_addr, size, dir);
> + unsigned long addr = (unsigned long)dma_to_cpu(dev, dma_addr);
> +
> + dma_sync_single_for_cpu(addr, size, dir);
> }
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH v2 2/2] ARM: set zero page accessible before copying ATAGs there
We used skip setting up the zero page as faulting when the SDRAM
starts at 0x0. One reason for doing that was that ATAGs will be copied
there in that case. Call zero_page_access() if necessary to be able
to set the zero page to faulting during barebox startup in the next
step.
Signed-off-by: Sascha Hauer
---
arch/arm/lib32/armlinux.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
index 6cb7d4b5f3..eb30f4a952 100644
--- a/arch/arm/lib32/armlinux.c
+++ b/arch/arm/lib32/armlinux.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -265,8 +266,12 @@ void start_linux(void *adr, int swap, unsigned long
initrd_address,
pr_debug("booting kernel with devicetree\n");
params = oftree;
} else {
- setup_tags(initrd_address, initrd_size, swap);
params = armlinux_get_bootparams();
+
+ if ((unsigned long)params < PAGE_SIZE)
+ zero_page_access();
+
+ setup_tags(initrd_address, initrd_size, swap);
}
architecture = armlinux_get_architecture();
--
2.39.2
[PATCH v2 1/2] scripts: omap3-usb-loader: fix heap overflow
From: Ahmad Fatoum Newer GCC versions correctly warn that the buffer allocated by realloc is too small. Correct the size. Signed-off-by: Ahmad Fatoum Link: https://lore.barebox.org/20230531062703.670521-3-ah...@a3f.at Signed-off-by: Sascha Hauer --- scripts/omap3-usb-loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/omap3-usb-loader.c b/scripts/omap3-usb-loader.c index a8d626c32f..31a03be8e7 100644 --- a/scripts/omap3-usb-loader.c +++ b/scripts/omap3-usb-loader.c @@ -784,7 +784,7 @@ int main(int argc, char *argv[]) file.addr = OMAP_BASE_ADDRESS; /* commit the file object with the processor specified base address */ - args->files = realloc(args->files, filecount); + args->files = realloc(args->files, filecount * sizeof(*args->files)); args->numfiles = filecount; args->files[filecount - 1] = malloc(sizeof (file)); memcpy(args->files[filecount - 1], , sizeof (file)); -- 2.39.2
[PATCH] Revert "dma: use dma/cpu conversions correctly in dma_map/unmap_single"
This reverts commit d13d870986eeecc58d8652268557e4a159b9d4c4.
While the patch itself is correct, it at least breaks USB on the
Raspberry Pi 3b.
With this patch dma_sync_single_for_device() is passed the DMA address.
This is correct as even the prototype suggests that it should get a
dma_addr_t. Unfortunately this is not what the function implements and
also not what the users expect. Most if not all users simply pass a CPU
pointer casted to unsigned long. dma_sync_single_for_device() on ARM
then takes the DMA address as a CPU pointer and does cache maintenance
on it.
Before we can merge this patch again dma_sync_single_for_device() must
get a struct device * argument and (on ARM) the cpu_to_dma() conversion
must be reverted before doing cache maintenance.
---
drivers/dma/map.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/map.c b/drivers/dma/map.c
index fea04c38a3..114c0f7db3 100644
--- a/drivers/dma/map.c
+++ b/drivers/dma/map.c
@@ -23,15 +23,17 @@ static inline void *dma_to_cpu(struct device *dev,
dma_addr_t addr)
dma_addr_t dma_map_single(struct device *dev, void *ptr, size_t size,
enum dma_data_direction dir)
{
- dma_addr_t ret = cpu_to_dma(dev, ptr);
+ unsigned long addr = (unsigned long)ptr;
- dma_sync_single_for_device(ret, size, dir);
+ dma_sync_single_for_device(addr, size, dir);
- return ret;
+ return cpu_to_dma(dev, ptr);
}
void dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
enum dma_data_direction dir)
{
- dma_sync_single_for_cpu(dma_addr, size, dir);
+ unsigned long addr = (unsigned long)dma_to_cpu(dev, dma_addr);
+
+ dma_sync_single_for_cpu(addr, size, dir);
}
--
2.39.2
Re: [PATCH 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
Hello Sascha,
On 31.05.23 12:01, Sascha Hauer wrote:
> We used to skip setting the zero page to faulting when SDRAM starts
> at 0x0. As bootm code now explicitly sets the zero page accessible
> before copying ATAGs there this should no longer be necessary, so
> unconditionally set the zero page to faulting during MMU startup.
> This also moves the zero page setup after the point the SDRAM has
> been mapped cachable, because otherwise the zero page setup would
> be overwritten.
>
> Signed-off-by: Sascha Hauer
> ---
> arch/arm/cpu/mmu_32.c | 21 ++---
> 1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
> index c4e5a3bb0a..faebba42d0 100644
> --- a/arch/arm/cpu/mmu_32.c
> +++ b/arch/arm/cpu/mmu_32.c
> @@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
> return -EINVAL;
> }
>
> -static void create_zero_page(void)
> -{
> - struct resource *zero_sdram;
> -
> - zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
I think this should remain to avoid some $platform overwriting ATAGS
or w/e with the kernel image. I know this is a discrepancy to ARM64,
but let's play it safe?
> - if (zero_sdram) {
> - /*
> - * Here we would need to set the second level page table
> - * entry to faulting. This is not yet implemented.
> - */
> - pr_debug("zero page is in SDRAM area, currently not
> supported\n");
> - } else {
> - zero_page_faulting();
> - pr_debug("Created zero page\n");
> - }
> -}
> -
> /*
> * Map vectors and zero page
> */
> @@ -487,7 +470,6 @@ static void vectors_init(void)
>*/
> if (!set_vector_table((unsigned long)__exceptions_start)) {
> arm_fixup_vectors();
> - create_zero_page();
> return;
> }
>
> @@ -495,7 +477,6 @@ static void vectors_init(void)
>* Next try high vectors at 0x.
>*/
> if (!set_vector_table(ARM_HIGH_VECTORS)) {
> - create_zero_page();
> create_vector_table(ARM_HIGH_VECTORS);
> return;
> }
> @@ -552,6 +533,8 @@ void __mmu_init(bool mmu_on)
>
> remap_range((void *)pos, bank->start + bank->size - pos,
> MAP_CACHED);
> }
> +
> + zero_page_faulting();
> }
>
> /*
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 1/2] ARM: set zero page accessible before copying ATAGs there
On 31.05.23 12:01, Sascha Hauer wrote:
> We used skip setting up the zero page as faulting when the SDRAM
> starts at 0x0. One reason for doing that was that ATAGs will be copied
> there in that case. Call zero_page_access() if necessary to be able
> to set the zero page to faulting during barebox startup in the next
> step.
>
> Signed-off-by: Sascha Hauer
> ---
> arch/arm/lib32/armlinux.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
> index 6cb7d4b5f3..9b2f1f4544 100644
> --- a/arch/arm/lib32/armlinux.c
> +++ b/arch/arm/lib32/armlinux.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -265,8 +266,12 @@ void start_linux(void *adr, int swap, unsigned long
> initrd_address,
> pr_debug("booting kernel with devicetree\n");
> params = oftree;
> } else {
> - setup_tags(initrd_address, initrd_size, swap);
> params = armlinux_get_bootparams();
> +
> + if ((unsigned long)params <= PAGE_SIZE)
> + zero_page_access();
Just < should be enough, no?
> +
> + setup_tags(initrd_address, initrd_size, swap);
> }
> architecture = armlinux_get_architecture();
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH 2/2] ARM: mmu_32: fix setting up zero page when it is in SDRAM
We used to skip setting the zero page to faulting when SDRAM starts
at 0x0. As bootm code now explicitly sets the zero page accessible
before copying ATAGs there this should no longer be necessary, so
unconditionally set the zero page to faulting during MMU startup.
This also moves the zero page setup after the point the SDRAM has
been mapped cachable, because otherwise the zero page setup would
be overwritten.
Signed-off-by: Sascha Hauer
---
arch/arm/cpu/mmu_32.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/mmu_32.c b/arch/arm/cpu/mmu_32.c
index c4e5a3bb0a..faebba42d0 100644
--- a/arch/arm/cpu/mmu_32.c
+++ b/arch/arm/cpu/mmu_32.c
@@ -459,23 +459,6 @@ static int set_vector_table(unsigned long adr)
return -EINVAL;
}
-static void create_zero_page(void)
-{
- struct resource *zero_sdram;
-
- zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
- if (zero_sdram) {
- /*
-* Here we would need to set the second level page table
-* entry to faulting. This is not yet implemented.
-*/
- pr_debug("zero page is in SDRAM area, currently not
supported\n");
- } else {
- zero_page_faulting();
- pr_debug("Created zero page\n");
- }
-}
-
/*
* Map vectors and zero page
*/
@@ -487,7 +470,6 @@ static void vectors_init(void)
*/
if (!set_vector_table((unsigned long)__exceptions_start)) {
arm_fixup_vectors();
- create_zero_page();
return;
}
@@ -495,7 +477,6 @@ static void vectors_init(void)
* Next try high vectors at 0x.
*/
if (!set_vector_table(ARM_HIGH_VECTORS)) {
- create_zero_page();
create_vector_table(ARM_HIGH_VECTORS);
return;
}
@@ -552,6 +533,8 @@ void __mmu_init(bool mmu_on)
remap_range((void *)pos, bank->start + bank->size - pos,
MAP_CACHED);
}
+
+ zero_page_faulting();
}
/*
--
2.39.2
[PATCH 1/2] ARM: set zero page accessible before copying ATAGs there
We used skip setting up the zero page as faulting when the SDRAM
starts at 0x0. One reason for doing that was that ATAGs will be copied
there in that case. Call zero_page_access() if necessary to be able
to set the zero page to faulting during barebox startup in the next
step.
Signed-off-by: Sascha Hauer
---
arch/arm/lib32/armlinux.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/lib32/armlinux.c b/arch/arm/lib32/armlinux.c
index 6cb7d4b5f3..9b2f1f4544 100644
--- a/arch/arm/lib32/armlinux.c
+++ b/arch/arm/lib32/armlinux.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -265,8 +266,12 @@ void start_linux(void *adr, int swap, unsigned long
initrd_address,
pr_debug("booting kernel with devicetree\n");
params = oftree;
} else {
- setup_tags(initrd_address, initrd_size, swap);
params = armlinux_get_bootparams();
+
+ if ((unsigned long)params <= PAGE_SIZE)
+ zero_page_access();
+
+ setup_tags(initrd_address, initrd_size, swap);
}
architecture = armlinux_get_architecture();
--
2.39.2
Re: [PATCH v2] Porting barebox to a new SoC
Hi Lior, On 31.05.23 10:05, Lior Weintraub wrote: > Hi Ahmad, > > Thanks again for your prompt reply and accurate tips! > Took the following changes: > 1. Increasing the DRAM size to 128MB (let barebox start from 0). > 2. Set PBL stack to offset 2MB from DRAM Just use end of SRAM, so you are completely independent of DRAM until you call barebox_arm_entry. > 3. Fix the device tree "memory" entry to have 128MB. (y) > 4. Load barebox-spider-mk1-evk.img to SRAM and run from there. > > Now I can see the following logs: > uncompress.c: memory at 0x, size 0x0800 > uncompress.c: uncompressing barebox binary at 0x00c021c0 (size > 0x0001e3a4) to 0x07e0 (uncompressed size: 0x000390d0) > uncompress.c: jumping to uncompressed image at 0x07e0 > start.c: memory at 0x, size 0x0800 > start.c: found DTB in boarddata, copying to 0x07dffcc0 > start.c: initializing malloc pool at 0x079ffcc0 (size 0x0040) > start.c: starting barebox... > initcall-> command_slice_init+0x0/0x3c > initcall-> globalvar_init+0x0/0x80 > register_device: global > register_device: nv > initcall-> platform_init+0x0/0x1c > register_device: platform > initcall-> amba_bus_init+0x0/0x1c > register_device: amba > initcall-> spi_bus_init+0x0/0x1c > register_device: spi > initcall-> gpio_desc_alloc+0x0/0x24 > initcall-> fs_bus_init+0x0/0x1c > register_device: fs > initcall-> aarch64_init_vectors+0x0/0x50 > initcall-> gpio_gate_clock_driver_register+0x0/0x1c > register_driver: gpio-gate-clock > initcall-> of_arm_init+0x0/0x5c > start.c: barebox_arm_boot_dtb: using barebox_boarddata > using boarddata provided DTB > adding DT alias:serial0: stem=serial id=0 node=/soc/serial@d000307000 > register_device: machine > of_platform_bus_create() - skipping /chosen, no compatible prop > of_platform_bus_create() - skipping /aliases, no compatible prop > of_platform_bus_create() - skipping /cpus, no compatible prop > of_platform_bus_create() - skipping /memory@0, no compatible prop > of_platform_bus_create() - skipping /soc, no compatible prop > initcall-> register_autoboot_vars+0x0/0x70 > initcall-> arm_arch_timer_driver_register+0x0/0x1c > register_driver: arm-architected-timer > initcall-> of_timer_init+0x0/0x20 > initcall-> init_fs+0x0/0x3c > initcall-> pl011_driver_register+0x0/0x1c > register_driver: uart-pl011 > initcall-> of_stdoutpath_init+0x0/0x28 > initcall-> of_probe_memory+0x0/0x60 > __request_region ok: 0x:0x07ff flags=0x0 > initcall-> __bare_init_end+0x0/0x6c > register_device: mem0 > initcall-> of_reserved_mem_walk+0x0/0x1ac > initcall-> mem_malloc_resource+0x0/0xa8 > __request_region ok: 0x079ffcc0:0x07dffcbf flags=0x200 > __request_region ok: 0x07e0:0x07e2aeff flags=0x200 > __request_region ok: 0x07e2af00:0x07e390cf flags=0x200 > __request_region ok: 0x07e390d0:0x07e3adaf flags=0x200 > initcall-> bootsource_init+0x0/0x40 > initcall-> ramfs_init+0x0/0x1c > register_driver: ramfs > initcall-> devfs_init+0x0/0x1c > register_driver: devfs > initcall-> arm_request_stack+0x0/0x398 > __request_region ok: 0x07ff:0x07ff7fff flags=0x200 > initcall-> mount_root+0x0/0x7c > mount: none on / type ramfs, options= > register_device: ramfs0 > probe-> ramfs0 > mount: none on /dev type devfs, options= > register_device: devfs0 > probe-> devfs0 > initcall-> binfmt_sh_init+0x0/0x1c > initcall-> binfmt_uimage_init+0x0/0x1c > initcall-> console_common_init+0x0/0x48 > initcall-> of_kernel_init+0x0/0x28 > initcall-> console_ctrlc_init+0x0/0x30 > initcall-> mem_init+0x0/0x90 > register_device: mem1 > register_driver: mem > probe-> mem0 > probe-> mem1 > initcall-> of_partition_init+0x0/0x48 > initcall-> prng_init+0x0/0x40 > initcall-> null_init+0x0/0x40 > initcall-> full_init+0x0/0x40 > initcall-> zero_init+0x0/0x40 > initcall-> spider_board_driver_register+0x0/0x1c > register_driver: board-spider > probe-> machine > initcall-> barebox_memory_areas_init+0x0/0x40 > __request_region ok: 0x07dffcc0:0x07dfffce flags=0x200 > initcall-> barebox_of_populate+0x0/0x28 > initcall-> of_register_memory_fixup+0x0/0x20 > initcall-> dummy_csrc_warn+0x0/0x44 > WARNING: Warning: Using dummy clocksource Add a arm,armv8-timer node into your SoC device tree. This lets barebox and Linux know that you have the ARM architected timer available. Without that, you'll notice that timeouts are wrong. > initcall-> bootm_init+0x0/0xf4 > initcall-> init_command_list+0x0/0x40 > register command bootm > register command cat > register command cd > register command clear > register command cp > register command cpuinfo > register command devinfo > register command drvinfo > register command echo > register command exit > register command false > register command help > register command ? > register command ll > register command ls > register command md > register command memcmp > register command memcpy > register command memset > register command mkdir > register command mount > register command mw > register
RE: [PATCH v2] Porting barebox to a new SoC
Hi Ahmad, Thanks again for your prompt reply and accurate tips! Took the following changes: 1. Increasing the DRAM size to 128MB (let barebox start from 0). 2. Set PBL stack to offset 2MB from DRAM 3. Fix the device tree "memory" entry to have 128MB. 4. Load barebox-spider-mk1-evk.img to SRAM and run from there. Now I can see the following logs: uncompress.c: memory at 0x, size 0x0800 uncompress.c: uncompressing barebox binary at 0x00c021c0 (size 0x0001e3a4) to 0x07e0 (uncompressed size: 0x000390d0) uncompress.c: jumping to uncompressed image at 0x07e0 start.c: memory at 0x, size 0x0800 start.c: found DTB in boarddata, copying to 0x07dffcc0 start.c: initializing malloc pool at 0x079ffcc0 (size 0x0040) start.c: starting barebox... initcall-> command_slice_init+0x0/0x3c initcall-> globalvar_init+0x0/0x80 register_device: global register_device: nv initcall-> platform_init+0x0/0x1c register_device: platform initcall-> amba_bus_init+0x0/0x1c register_device: amba initcall-> spi_bus_init+0x0/0x1c register_device: spi initcall-> gpio_desc_alloc+0x0/0x24 initcall-> fs_bus_init+0x0/0x1c register_device: fs initcall-> aarch64_init_vectors+0x0/0x50 initcall-> gpio_gate_clock_driver_register+0x0/0x1c register_driver: gpio-gate-clock initcall-> of_arm_init+0x0/0x5c start.c: barebox_arm_boot_dtb: using barebox_boarddata using boarddata provided DTB adding DT alias:serial0: stem=serial id=0 node=/soc/serial@d000307000 register_device: machine of_platform_bus_create() - skipping /chosen, no compatible prop of_platform_bus_create() - skipping /aliases, no compatible prop of_platform_bus_create() - skipping /cpus, no compatible prop of_platform_bus_create() - skipping /memory@0, no compatible prop of_platform_bus_create() - skipping /soc, no compatible prop initcall-> register_autoboot_vars+0x0/0x70 initcall-> arm_arch_timer_driver_register+0x0/0x1c register_driver: arm-architected-timer initcall-> of_timer_init+0x0/0x20 initcall-> init_fs+0x0/0x3c initcall-> pl011_driver_register+0x0/0x1c register_driver: uart-pl011 initcall-> of_stdoutpath_init+0x0/0x28 initcall-> of_probe_memory+0x0/0x60 __request_region ok: 0x:0x07ff flags=0x0 initcall-> __bare_init_end+0x0/0x6c register_device: mem0 initcall-> of_reserved_mem_walk+0x0/0x1ac initcall-> mem_malloc_resource+0x0/0xa8 __request_region ok: 0x079ffcc0:0x07dffcbf flags=0x200 __request_region ok: 0x07e0:0x07e2aeff flags=0x200 __request_region ok: 0x07e2af00:0x07e390cf flags=0x200 __request_region ok: 0x07e390d0:0x07e3adaf flags=0x200 initcall-> bootsource_init+0x0/0x40 initcall-> ramfs_init+0x0/0x1c register_driver: ramfs initcall-> devfs_init+0x0/0x1c register_driver: devfs initcall-> arm_request_stack+0x0/0x398 __request_region ok: 0x07ff:0x07ff7fff flags=0x200 initcall-> mount_root+0x0/0x7c mount: none on / type ramfs, options= register_device: ramfs0 probe-> ramfs0 mount: none on /dev type devfs, options= register_device: devfs0 probe-> devfs0 initcall-> binfmt_sh_init+0x0/0x1c initcall-> binfmt_uimage_init+0x0/0x1c initcall-> console_common_init+0x0/0x48 initcall-> of_kernel_init+0x0/0x28 initcall-> console_ctrlc_init+0x0/0x30 initcall-> mem_init+0x0/0x90 register_device: mem1 register_driver: mem probe-> mem0 probe-> mem1 initcall-> of_partition_init+0x0/0x48 initcall-> prng_init+0x0/0x40 initcall-> null_init+0x0/0x40 initcall-> full_init+0x0/0x40 initcall-> zero_init+0x0/0x40 initcall-> spider_board_driver_register+0x0/0x1c register_driver: board-spider probe-> machine initcall-> barebox_memory_areas_init+0x0/0x40 __request_region ok: 0x07dffcc0:0x07dfffce flags=0x200 initcall-> barebox_of_populate+0x0/0x28 initcall-> of_register_memory_fixup+0x0/0x20 initcall-> dummy_csrc_warn+0x0/0x44 WARNING: Warning: Using dummy clocksource initcall-> bootm_init+0x0/0xf4 initcall-> init_command_list+0x0/0x40 register command bootm register command cat register command cd register command clear register command cp register command cpuinfo register command devinfo register command drvinfo register command echo register command exit register command false register command help register command ? register command ll register command ls register command md register command memcmp register command memcpy register command memset register command mkdir register command mount register command mw register command pwd register command rm register command rmdir register command setenv register command sh register command source register command . register command test register command [ register command true register command : register command umount register command version initcall-> display_meminfo+0x0/0xa8 barebox code: 0x7e0 -> 0x7e2aeff bss segment: 0x7e390d0 -> 0x7e3adaf malloc space: 0x079ffcc0 -> 0x07dffcbf (size 4 MiB) initcall-> of_register_bootargs_fixup+0x0/0x3c initcall-> device_probe_deferred+0x0/0x14c initcall-> of_init_hostname+0x0/0x28 initcall->
Re: [PATCH master 1/3] ARM: rockchip: pine64-quartz64: add sdram-init.bin to .gitignore
On Wed, May 31, 2023 at 08:27:01AM +0200, Ahmad Fatoum wrote: > Other boards already have sdram-init.bin in their .gitignore, so have > quartz64 follow suit. > > Signed-off-by: Ahmad Fatoum > --- > arch/arm/boards/pine64-quartz64/.gitignore | 1 + > 1 file changed, 1 insertion(+) > create mode 100644 arch/arm/boards/pine64-quartz64/.gitignore Applied, thanks Sascha > > diff --git a/arch/arm/boards/pine64-quartz64/.gitignore > b/arch/arm/boards/pine64-quartz64/.gitignore > new file mode 100644 > index ..f458f794b54c > --- /dev/null > +++ b/arch/arm/boards/pine64-quartz64/.gitignore > @@ -0,0 +1 @@ > +sdram-init.bin > -- > 2.38.5 > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 1/2] efi: payload: iomem: add commented out #define DEBUG 1
On Wed, May 31, 2023 at 12:38:20AM +0900, Masahiro Yamada wrote:
> On Tue, May 30, 2023 at 9:38 PM Sascha Hauer wrote:
> >
> > Picked some outdated address from Masahiro, so once again:
>
>
> The macro name is misleading - perhaps, it should have
> been named as __is_defined_as_1().
>
> I do not know if __is_defined() is future-proof.
> IS_BUILTIN, IS_MODULE, etc. are official,
> but __is_defined() is internal.
Yes, it looks like __is_defined() is meant to be used internally by
Kconfig, nevertheless it's getting more users in the kernel:
arch/arm64/include/asm/cpufeature.h:408:return
__is_defined(__KVM_VHE_HYPERVISOR__);
arch/arm64/include/asm/cpufeature.h:414:return
__is_defined(__KVM_NVHE_HYPERVISOR__);
arch/arm64/include/asm/kvm_nested.h:9: return
(!__is_defined(__KVM_NVHE_HYPERVISOR__) &&
arch/powerpc/include/asm/vdso/timebase.h:54:if (__is_defined(__powerpc64__))
arch/s390/include/asm/nospec-branch.h:17: return
__is_defined(CC_USING_EXPOLINE) && !nospec_disable;
arch/s390/kernel/nospec-branch.c:67:if
(__is_defined(CC_USING_EXPOLINE))
arch/s390/kernel/nospec-branch.c:70:} else if
(__is_defined(CC_USING_EXPOLINE)) {
drivers/crypto/sahara.c:356:if (!__is_defined(DEBUG))
drivers/crypto/sahara.c:408:if (!__is_defined(DEBUG))
drivers/crypto/sahara.c:429:if (!__is_defined(DEBUG))
drivers/usb/host/sl811-hcd.c:1290: if (__is_defined(VERBOSE) ||
include/linux/pgtable.h:1605:#define mm_p4d_folded(mm)
__is_defined(__PAGETABLE_P4D_FOLDED)
include/linux/pgtable.h:1609:#define mm_pud_folded(mm)
__is_defined(__PAGETABLE_PUD_FOLDED)
include/linux/pgtable.h:1613:#define mm_pmd_folded(mm)
__is_defined(__PAGETABLE_PMD_FOLDED)
security/smack/smack_lsm.c:2911:if
(__is_defined(SMACK_IPV6_SECMARK_LABELING))
Maybe we should move __is_defined() somewhere else so that it can
officially be used.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] regulator: core: add debug print for regulator_resolve_supply
Starting with commit 324bd9bbe7e8 ("regulator: recursively enable/disable
regulator dependency tree"), regulator operations may affect more than
just the one regulator being enabled. Place a debug print, so it's
easier to follow the dependency chain.
Signed-off-by: Ahmad Fatoum
---
drivers/regulator/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 42774a561ce5..f914641c8777 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -141,6 +141,8 @@ static int regulator_resolve_supply(struct regulator_dev
*rdev)
if (!supply_name)
return 0;
+ dev_dbg(rdev->dev, "resolving %s\n", supply_name);
+
supply = regulator_get(rdev->dev, supply_name);
if (IS_ERR(supply)) {
if (deep_probe_is_supported())
--
2.38.5
[PATCH 1/3] include: sync min/max definitions with Linux
By syncing to Linux, min/max and friends now return constant expressions
if possible, which makes them suitable for use as non-VLA array length.
Signed-off-by: Ahmad Fatoum
---
include/linux/const.h | 9 +++
include/linux/kernel.h | 123 +-
include/linux/minmax.h | 169 +
3 files changed, 179 insertions(+), 122 deletions(-)
create mode 100644 include/linux/minmax.h
diff --git a/include/linux/const.h b/include/linux/const.h
index 07f886d27155..0acddd3a0ed6 100644
--- a/include/linux/const.h
+++ b/include/linux/const.h
@@ -26,4 +26,13 @@
#define _BITUL(x) (_AC(1,UL) << (x))
#define _BITULL(x) (_AC(1,ULL) << (x))
+
+/*
+ * This returns a constant expression while determining if an argument is
+ * a constant expression, most importantly without evaluating the argument.
+ * Glory to Martin Uecker
+ */
+#define __is_constexpr(x) \
+ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+
#endif /* !(_LINUX_CONST_H) */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 0eb7bd21fe93..bf820de22ca7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#define ALIGN(x, a)__ALIGN_MASK(x, (typeof(x))(a) - 1)
#define ALIGN_DOWN(x, a) ALIGN((x) - ((a) - 1), (a))
@@ -88,121 +89,6 @@ extern long simple_strtol(const char *,char **,unsigned
int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
-/*
- * min()/max()/clamp() macros that also do
- * strict type-checking.. See the
- * "unnecessary" pointer comparison.
- */
-#define min(x, y) ({ \
- typeof(x) _min1 = (x); \
- typeof(y) _min2 = (y); \
- (void) (&_min1 == &_min2); \
- _min1 < _min2 ? _min1 : _min2; })
-
-#define max(x, y) ({ \
- typeof(x) _max1 = (x); \
- typeof(y) _max2 = (y); \
- (void) (&_max1 == &_max2); \
- _max1 > _max2 ? _max1 : _max2; })
-
-#define min3(x, y, z) ({ \
- typeof(x) _min1 = (x); \
- typeof(y) _min2 = (y); \
- typeof(z) _min3 = (z); \
- (void) (&_min1 == &_min2); \
- (void) (&_min1 == &_min3); \
- _min1 < _min2 ? (_min1 < _min3 ? _min1 : _min3) : \
- (_min2 < _min3 ? _min2 : _min3); })
-
-#define max3(x, y, z) ({ \
- typeof(x) _max1 = (x); \
- typeof(y) _max2 = (y); \
- typeof(z) _max3 = (z); \
- (void) (&_max1 == &_max2); \
- (void) (&_max1 == &_max3); \
- _max1 > _max2 ? (_max1 > _max3 ? _max1 : _max3) : \
- (_max2 > _max3 ? _max2 : _max3); })
-
-/**
- * min_not_zero - return the minimum that is _not_ zero, unless both are zero
- * @x: value1
- * @y: value2
- */
-#define min_not_zero(x, y) ({ \
- typeof(x) __x = (x);\
- typeof(y) __y = (y);\
- __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
-
-/**
- * clamp - return a value clamped to a given range with strict typechecking
- * @val: current value
- * @min: minimum allowable value
- * @max: maximum allowable value
- *
- * This macro does strict typechecking of min/max to make sure they are of the
- * same type as val. See the unnecessary pointer comparisons.
- */
-#define clamp(val, min, max) ({\
- typeof(val) __val = (val); \
- typeof(min) __min = (min); \
- typeof(max) __max = (max); \
- (void) (&__val == &__min); \
- (void) (&__val == &__max); \
- __val = __val < __min ? __min: __val; \
- __val > __max ? __max: __val; })
-
-/*
- * ..and if you can't take the strict
- * types, you can specify one yourself.
- *
- * Or not use min/max/clamp at all, of course.
- */
-#define min_t(type, x, y) ({ \
- type __min1 = (x); \
- type __min2 = (y); \
- __min1 < __min2 ? __min1: __min2; })
-
-#define max_t(type, x, y) ({ \
- type __max1 = (x); \
- type __max2 = (y); \
- __max1 > __max2 ? __max1: __max2; })
-
-/**
- * clamp_t - return a value clamped to a given range using a given type
- * @type: the type of variable to use
- * @val: current value
- * @min: minimum allowable value
- * @max: maximum allowable value
- *
- * This macro does no typechecking and uses temporary variables of type
- * 'type' to make all the comparisons.
[PATCH 3/3] string: import strverscmp_improved from systemd
The Boot Loader specification now references the UAPI group's version
format specification[1] on how blspec entries should be sorted.
In preparation of aligning barebox entry sorting with the specification,
import systemd's strverscmp_improved as strverscmp and add some tests
for it.
The selftest is called string.c, because it indirectly tests some string
mangling function and in anticipation of adding more string tests in the
future.
[1]:
https://uapi-group.org/specifications/specs/version_format_specification/#examples
Signed-off-by: Ahmad Fatoum
---
include/string.h | 2 +
lib/Kconfig| 9 ++-
lib/Makefile | 1 +
lib/strverscmp.c | 165 ++
test/self/Kconfig | 6 ++
test/self/Makefile | 1 +
test/self/string.c | 175 +
7 files changed, 358 insertions(+), 1 deletion(-)
create mode 100644 lib/strverscmp.c
create mode 100644 test/self/string.c
diff --git a/include/string.h b/include/string.h
index 499f2ec03c02..43911b75762f 100644
--- a/include/string.h
+++ b/include/string.h
@@ -18,4 +18,6 @@ void *__nokasan_default_memcpy(void * dest,const void
*src,size_t count);
char *parse_assignment(char *str);
+int strverscmp(const char *a, const char *b);
+
#endif /* __STRING_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index b8bc9d63d4f0..84d2a2573625 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -107,7 +107,7 @@ config IMAGE_SPARSE
bool
config STMP_DEVICE
- bool "STMP device support" if COMPILE_TEST
+ bool "STMP device support"
config FSL_QE_FIRMWARE
select CRC32
@@ -167,6 +167,13 @@ config PROGRESS_NOTIFIER
This is selected by boards that register a notifier to visualize
progress, like blinking a LED during an update.
+config VERSION_CMP
+ bool "version comparison utilities" if COMPILE_TEST
+ help
+ This is selected by code that needs to compare versions
+ in a manner compatible with
+
https://uapi-group.org/specifications/specs/version_format_specification
+
config PRINTF_UUID
bool
default y if PRINTF_FULL
diff --git a/lib/Makefile b/lib/Makefile
index 38478625423b..185e6221fdd2 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -6,6 +6,7 @@ obj-pbl-y += ctype.o
obj-y += rbtree.o
obj-y += display_options.o
obj-y += string.o
+obj-$(CONFIG_VERSION_CMP) += strverscmp.o
obj-y += strtox.o
obj-y += kstrtox.o
obj-y += vsprintf.o
diff --git a/lib/strverscmp.c b/lib/strverscmp.c
new file mode 100644
index ..da2d284918e0
--- /dev/null
+++ b/lib/strverscmp.c
@@ -0,0 +1,165 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Code taken from systemd src/fundamental/string-util-fundamental.c
+ * NOTE: Semantics differ from glibc strverscmp (e.g. handling of ~rc1)
+ */
+
+#include
+#include
+#include
+
+static bool is_valid_version_char(char a)
+{
+return isdigit(a) || isalpha(a) || a == '~' ||
+ a == '-' || a == '^' || a == '.';
+}
+
+int strverscmp(const char *a, const char *b)
+{
+/* This function is similar to strverscmp(3), but it treats '-' and
'.' as separators.
+ *
+ * The logic is based on rpm's rpmvercmp(), but unlike rpmvercmp(), it
distiguishes e.g.
+ * '123a' and '123.a', with '123a' being newer.
+ *
+ * It allows direct comparison of strings which contain both a version
and a release; e.g.
+ * '247.2-3.1.fc33.x86_64' or
'5.11.0-0.rc5.20210128git76c057c84d28.137.fc34'.
+ *
+ * The input string is split into segments. Each segment is numeric or
alphabetic, and may be
+ * prefixed with the following:
+ * '~' : used for pre-releases, a segment prefixed with this is the
oldest,
+ * '-' : used for the separator between version and release,
+ * '^' : used for patched releases, a segment with this is newer than
one with '-'.
+ * '.' : used for point releases.
+ * Note that no prefix segment is the newest. All non-supported
characters are dropped, and
+ * handled as a separator of segments, e.g., '123_a' is equivalent to
'123a'.
+ *
+ * By using this, version strings can be sorted like following:
+ * (older) 122.1
+ * ^123~rc1-1
+ * |123
+ * |123-a
+ * |123-a.1
+ * |123-1
+ * |123-1.1
+ * |123^post1
+ * |123.a-1
+ * |123.1-1
+ * v123a-1
+ * (newer) 124-1
+ */
+
+a = a ?: "";
+b = b ?: "";
+
+for (;;) {
+const char *aa, *bb;
+int r;
+
+/* Drop leading invalid characters. */
+while (*a != '\0' &&
[PATCH 2/3] include: minmax.h: implement compare3 helper
Define the macro for use in comparison functions. The macro has the same
semantics as systemd's CMP() macro with the difference that it returns
constant expressions if possible.
Signed-off-by: Ahmad Fatoum
---
include/linux/minmax.h | 20
1 file changed, 20 insertions(+)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 396df1121bff..9b6ddad7b3f6 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -166,4 +166,24 @@
#define swap(a, b) \
do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
+
+#define __compare3(x, y) ((x) < (y) ? -1 : (x) > (y) ? 1 : 0)
+
+#define __compare3_once(x, y, unique_x, unique_y) ({ \
+ typeof(x) unique_x = (x); \
+ typeof(y) unique_y = (y); \
+ __compare3(unique_x, unique_y); })
+
+/**
+ * compare3 - 3-way comparison of @x and @y
+ * @x: first value
+ * @y: second value
+ *
+ * returns -1 if x < y, 0 if x == y and 1 if x > y
+ */
+#define compare3(x, y) \
+ __builtin_choose_expr(__safe_cmp(x, y), \
+ __compare3(x, y), \
+ __compare3_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y)))
+
#endif /* _LINUX_MINMAX_H */
--
2.38.5
[PATCH 0/3] string: import strverscmp_improved from systemd
The Boot Loader specification now references the UAPI group's version format specification[1] on how blspec entries should be sorted. In preparation of aligning barebox entry sorting with the specification, import systemd's strverscmp_improved as strverscmp and add some tests for it. The changes to bootloader spec sorting need some more testing and will follow separately. Ahmad Fatoum (3): include: sync min/max definitions with Linux include: minmax.h: implement compare3 helper string: import strverscmp_improved from systemd include/linux/const.h | 9 ++ include/linux/kernel.h | 123 +-- include/linux/minmax.h | 189 + include/string.h | 2 + lib/Kconfig| 9 +- lib/Makefile | 1 + lib/strverscmp.c | 165 +++ test/self/Kconfig | 6 ++ test/self/Makefile | 1 + test/self/string.c | 175 ++ 10 files changed, 557 insertions(+), 123 deletions(-) create mode 100644 include/linux/minmax.h create mode 100644 lib/strverscmp.c create mode 100644 test/self/string.c -- 2.38.5
[PATCH master 3/3] scripts: omap3-usb-loader: fix heap overflow
Newer GCC versions correctly warn that the buffer allocated by realloc is too small. Correct the size. Signed-off-by: Ahmad Fatoum --- scripts/omap3-usb-loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/omap3-usb-loader.c b/scripts/omap3-usb-loader.c index a8d626c32f23..31a03be8e7f4 100644 --- a/scripts/omap3-usb-loader.c +++ b/scripts/omap3-usb-loader.c @@ -784,7 +784,7 @@ int main(int argc, char *argv[]) file.addr = OMAP_BASE_ADDRESS; /* commit the file object with the processor specified base address */ - args->files = realloc(args->files, filecount); + args->files = realloc(args->files, filecount * sizeof(*args->files)); args->numfiles = filecount; args->files[filecount - 1] = malloc(sizeof (file)); memcpy(args->files[filecount - 1], , sizeof (file)); -- 2.38.5
[PATCH master 1/3] ARM: rockchip: pine64-quartz64: add sdram-init.bin to .gitignore
Other boards already have sdram-init.bin in their .gitignore, so have quartz64 follow suit. Signed-off-by: Ahmad Fatoum --- arch/arm/boards/pine64-quartz64/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 arch/arm/boards/pine64-quartz64/.gitignore diff --git a/arch/arm/boards/pine64-quartz64/.gitignore b/arch/arm/boards/pine64-quartz64/.gitignore new file mode 100644 index ..f458f794b54c --- /dev/null +++ b/arch/arm/boards/pine64-quartz64/.gitignore @@ -0,0 +1 @@ +sdram-init.bin -- 2.38.5
[PATCH master 2/3] scripts: kwbimage: check return value of asprintf
Some newer toolchains defines asprintf with a must_check attribute,
leading to warnings when compiling kwbimage. Let's handle OOM gracefully
to get rid of the warnings.
Signed-off-by: Ahmad Fatoum
---
scripts/kwbimage.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
index f9d052752d79..370c54c983b5 100644
--- a/scripts/kwbimage.c
+++ b/scripts/kwbimage.c
@@ -1006,6 +1006,7 @@ static int image_create_config_parse_oneline(char *line,
char *configpath)
{
char *keyword, *saveptr;
+ int ret;
keyword = strtok_r(line, " ", );
if (!strcmp(keyword, "VERSION")) {
@@ -1056,10 +1057,16 @@ static int image_create_config_parse_oneline(char *line,
int argi = 0;
el->type = IMAGE_CFG_BINARY;
- if (*value == '/')
+ if (*value == '/') {
el->binary.file = strdup(value);
- else
- asprintf(>binary.file, "%s/%s", configpath, value);
+ } else {
+ ret = asprintf(>binary.file, "%s/%s", configpath,
value);
+ if (ret < 0) {
+ fprintf(stderr, "Cannot allocate memory\n");
+ return -1;
+ }
+ }
+
while (1) {
value = strtok_r(NULL, " ", );
if (!value)
--
2.38.5
Re: [PATCH v2] Porting barebox to a new SoC
Hi Lior,
On 30.05.23 22:10, Lior Weintraub wrote:
> Hello Ahmad,
>
> Thanks again for your kind support!
> Your comments helped me progress and the current situation is as follows:
> Our QEMU Spider machine is running a BL1.elf file using the following command:
> QEMU_AUDIO_DRV=none qemu-system-aarch64 -M spider-soc -smp 1 -m 128M
> -nographic \
> -device loader,file=BL1.elf -cpu cortex-a53,rvbar=0xC00400 \
> -d unimp -semihosting-config enable=on,target=native \
> -monitor telnet:localhost:1235,server,nowait \
> -gdb tcp::1236
>
> The BL1.elf is our BootROM application that runs from ROM address
> 0xC00400.
> Just for debugging purpose we've increased the ROM size so it can include the
> images/barebox-spider-mk1-evk.img (as a const array).
> BL1 then copy it (using memcpy) to address 0 and jumps there for execution.
Sounds ok.
> On the real ASIC this address will be the DRAM and indeed will need to be
> initialized before the copy but here on QEMU it is not required.
I see. You could still have your bootrom move barebox into SRAM and then
barebox_arm_entry(DRAM_ADDR, SZ_128M, __dtb_spider_mk1_evk_start);
To have PBL extract barebox to DRAM. Even if you don't need need DRAM
setup in QEMU, the flow would be similar to what you will have on actual
silicon.
> The lowlevel.c of spider-evk was modified to use DRAM_ADDR 0x40 (4MB from
> start) and MY_STACK_TOP was set to 0x800 (128M).
That's not needed. While you don't have control where barebox PBL will be
located
(depends on BootROM), barebox will extract itself to the end of DRAM without
overwriting itself, so you can just use DRAM_ADDR 0 normally. Eventually, stack
top should go into SRAM, but anywhere that works is ok.
> In addition, I implemented putc_ll (currently hard-coded in
> include/debug_ll.h) and opened log level to VERBOSE_DEBUG (currently just
> hard-coded in printk.h).
There's CONFIG_DEBUG_PBL you can enable to get all these messages by default.
> I see the following (which makes sense) logs on QEMU terminal:
> uncompress.c: memory at 0x0040, size 0x0020
> uncompress.c: uncompressing barebox binary at 0x2200 (size
> 0x0001d3b2) to 0x0040 (uncompressed size: 0x000373d0)
Why pass only 2M to barebox_arm_entry? While this need not be the whole of RAM,
this
initial memory is what barebox will use for itself including its final stack and
malloc area. barebox will also not place itself into the last 1M AFAIK, so 2M
may not work ok. You should use the minimum possible RAM here or if you can
detect
in PBL how much RAM you have or just the whole RAM bank. I am not sure anything
lower
than SZ_4M would work ok. (Note this is DRAM size, not SRAM. barebox PBL is
fine being
called from 64K SRAMs, it just needs DRAM to extract to).
> I then connect with aarch64-none-elf-gdb, load the barebox symbols (to
> 0x40) and check the current execution state (program counter and stack).
> Looks like the code is stuck on function register_autoboot_vars:
> sp 0x5f7e600x5f7e60
> pc 0x4012640x401264
>
> Few observations:
> 1. The PBL was using stack top located on 0x800 (which is MY_STACK_TOP).
> Found it be causing a breakpoint on the PBL.
> 2. Barebox uses a stack top on 0x60 which is probably calculated from my
> new DRAM_ADDR and SZ_2M given to the function call:
> barebox_arm_entry(DRAM_ADDR, SZ_2M, __dtb_spider_mk1_evk_start);
>
> This is great! I am starting to find my way.
:) Ye, the ENTRY_FUNCTION_WITHSTACK is only when the BootROM doesn't enter
with a valid stack pointer. It's overwritten as soon as barebox is told
about the initial memory layout (with barebox_arm_entry).
> When the barebox execution didn't print anything to the terminal I remembered
> that we used a place holder on the dtsi for the uart.
> So now I changed it to:
> uart0: serial@d000307000 {
>compatible = "arm,pl011", "arm,primecell";
>reg = <0xd0 0x307000 0 0x1000>;
> }
> (Our QEMU UART is currently using pl011 device.)
Looks ok. Don't forget to enable the driver (via make menuconfig).
> After some time (trying to debug by enabling MMU but then reverted the code
> back), I got to a point that when I try to run again I am getting an
> exception.
> I can swear all code changes were reverted back to the point where I saw the
> barebox stuck on register_autoboot_vars but now it just cases an exception.
> The exception vector code is located on our ROM (part of BL1 code) and the
> logs shows that the link register has the value 0x401218 which suggest the
> following code:
> 1200 :
> 1200: a9be7bfdstp x29, x30, [sp, #-32]!
> 1204: 910003fdmov x29, sp
> 1208: a90153f3stp x19, x20, [sp, #16]
> 120c: 94000a07bl 3a28
> 1210: d0f3adrpx19, 1f000
> 1214: 912a7273add x19, x19, #0xa9c
> 1218: aa0003f4mov x20, x0
>
>
