[PATCH 1/4] driver: left shift 1U instead of 1 in bitmask for consistency

[PATCH 1/4] driver: left shift 1U instead of 1 in bitmask for consistency

[Ahmad Fatoum]

The first two masks use 1U already, so follow suit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/driver.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/driver.h b/include/driver.h
index 1b61f2066099..d509031f4980 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -489,8 +489,8 @@ int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
 
 #define DEVFS_PARTITION_FIXED		(1U << 0)
 #define DEVFS_PARTITION_READONLY	(1U << 1)
-#define DEVFS_IS_CHARACTER_DEV		(1 << 3)
-#define DEVFS_PARTITION_FROM_TABLE	(1 << 4)
+#define DEVFS_IS_CHARACTER_DEV		(1U << 3)
+#define DEVFS_PARTITION_FROM_TABLE	(1U << 4)
 
 struct cdev *devfs_add_partition(const char *devname, loff_t offset,
 		loff_t size, unsigned int flags, const char *name);
-- 
2.19.1


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

[PATCH 2/4] of: partition: use NULL instead of never-read argument

[Ahmad Fatoum]

len isn't read afterward and of_get_property accepts NULL as third
argument, so do it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/partition.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index aa6e601b7fe4..257100112108 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -64,9 +64,9 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct device_node *node)
 	offset = of_read_number(reg, na);
 	size = of_read_number(reg + na, ns);
 
-	partname = of_get_property(node, "label", &len);
+	partname = of_get_property(node, "label", NULL);
 	if (!partname)
-		partname = of_get_property(node, "name", &len);
+		partname = of_get_property(node, "name", NULL);
 	if (!partname)
 		return NULL;
 
@@ -74,7 +74,7 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct device_node *node)
 
 	debug("add partition: %s.%s 0x%08llx 0x%08llx\n", cdev->name, partname, offset, size);
 
-	if (of_get_property(node, "read-only", &len))
+	if (of_get_property(node, "read-only", NULL))
 		flags = DEVFS_PARTITION_READONLY;
 
 	filename = basprintf("%s.%s", cdev->name, partname);
-- 
2.19.1


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

[PATCH 3/4] of: partitions: make macro name more generic

[Ahmad Fatoum]

DEVFS_PARTITION_FROM_TABLE's only effect at the moment is to disable
device tree fixups. Rename it accordingly, so it looks less out-of-place
when using it in the next commit to implement the no-fixup property.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions.c    | 2 +-
 drivers/of/partition.c | 4 ++--
 include/driver.h       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/partitions.c b/common/partitions.c
index 574b31fbbe9b..cb4e0c8c133e 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -63,7 +63,7 @@ static int register_one_partition(struct block_device *blk,
 		goto out;
 	}
 
-	cdev->flags |= DEVFS_PARTITION_FROM_TABLE;
+	cdev->flags |= DEVFS_PARTITION_NO_FIXUP;
 
 	cdev->dos_partition_type = part->dos_partition_type;
 	strcpy(cdev->partuuid, part->partuuid);
diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index 257100112108..450dda17dfd7 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -141,7 +141,7 @@ static int of_partition_fixup(struct device_node *root, void *ctx)
 		return -EINVAL;
 
 	list_for_each_entry(partcdev, &cdev->partitions, partition_entry) {
-		if (partcdev->flags & DEVFS_PARTITION_FROM_TABLE)
+		if (partcdev->flags & DEVFS_PARTITION_NO_FIXUP)
 			continue;
 		n_parts++;
 	}
@@ -201,7 +201,7 @@ static int of_partition_fixup(struct device_node *root, void *ctx)
 		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_NO_FIXUP)
 			continue;
 
 		if (partcdev->mtd)
diff --git a/include/driver.h b/include/driver.h
index d509031f4980..ba2dc209c239 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -490,7 +490,7 @@ int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
 #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_PARTITION_NO_FIXUP	(1U << 4)
 
 struct cdev *devfs_add_partition(const char *devname, loff_t offset,
 		loff_t size, unsigned int flags, const char *name);
-- 
2.19.1


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

[PATCH 4/4] of: partition: add no-fixup device tree property

[Ahmad Fatoum]

Users can have reasons to inhibit barebox from fixing up partitions nodes
into the kernel device tree, like if it's a boot0-partitions node, which
doesn't have a kernel binding.
Allow users to specify a no-fixup property to indicate so.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/devicetree/bindings/mtd/partition.txt | 3 +++
 drivers/of/partition.c                              | 9 +++++++++
 fs/devfs-core.c                                     | 1 +
 3 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
index 4288a82437b4..a2e810276a9c 100644
--- a/Documentation/devicetree/bindings/mtd/partition.txt
+++ b/Documentation/devicetree/bindings/mtd/partition.txt
@@ -4,6 +4,8 @@ In addition to the upstream binding, another property is added:
 
 Optional properties:
 - partuuid : The partition UUID for this partition.
+- no-fixup : empty property, indicates that barebox shouldn't fix up the
+             containing node into the kernel device tree.
 
 Additionally, barebox also supports partitioning the eMMC boot partitions if
 the partition table node is named appropriately:
@@ -28,6 +30,7 @@ flash@0 {
 emmc@1 {
 	boot0-partitions {
 		compatible = "fixed-partitions";
+		no-fixup;
 		#address-cells = <1>;
 		#size-cells = <1>;
 
diff --git a/drivers/of/partition.c b/drivers/of/partition.c
index 450dda17dfd7..219a91eeff44 100644
--- a/drivers/of/partition.c
+++ b/drivers/of/partition.c
@@ -77,6 +77,9 @@ struct cdev *of_parse_partition(struct cdev *cdev, struct device_node *node)
 	if (of_get_property(node, "read-only", NULL))
 		flags = DEVFS_PARTITION_READONLY;
 
+	if (of_get_property(node, "no-fixup", NULL))
+		flags |= DEVFS_PARTITION_NO_FIXUP;
+
 	filename = basprintf("%s.%s", cdev->name, partname);
 
 	new = devfs_add_partition(cdev->name, offset, size, flags, filename);
@@ -107,6 +110,9 @@ int of_parse_partitions(struct cdev *cdev, struct device_node *node)
 		node = subnode;
 	}
 
+	if (of_get_property(node, "no-fixup", NULL))
+		cdev->flags |= DEVFS_PARTITION_NO_FIXUP;
+
 	for_each_child_of_node(node, n) {
 		of_parse_partition(cdev, n);
 	}
@@ -247,6 +253,9 @@ static int of_partition_fixup(struct device_node *root, void *ctx)
 
 int of_partitions_register_fixup(struct cdev *cdev)
 {
+	if (cdev->flags & DEVFS_PARTITION_NO_FIXUP)
+		return 0;
+
 	return of_register_fixup(of_partition_fixup, cdev);
 }
 
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index f017e1c55d4c..19cea8a7ce56 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -339,6 +339,7 @@ static struct cdev *__devfs_add_partition(struct cdev *cdev,
 	new->priv = cdev->priv;
 	new->size = size;
 	new->offset = cdev->offset + offset;
+	new->flags = partinfo->flags;
 
 	new->dev = cdev->dev;
 	new->master = cdev;
-- 
2.19.1


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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Sascha Hauer]

On Fri, Dec 07, 2018 at 07:34:37AM +0100, Ahmad Fatoum wrote:
> Users can have reasons to inhibit barebox from fixing up partitions nodes
> into the kernel device tree, like if it's a boot0-partitions node, which
> doesn't have a kernel binding.
> Allow users to specify a no-fixup property to indicate so.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/mtd/partition.txt | 3 +++
>  drivers/of/partition.c                              | 9 +++++++++
>  fs/devfs-core.c                                     | 1 +
>  3 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> index 4288a82437b4..a2e810276a9c 100644
> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> @@ -4,6 +4,8 @@ In addition to the upstream binding, another property is added:
>  
>  Optional properties:
>  - partuuid : The partition UUID for this partition.
> +- no-fixup : empty property, indicates that barebox shouldn't fix up the
> +             containing node into the kernel device tree.
>  
>  Additionally, barebox also supports partitioning the eMMC boot partitions if
>  the partition table node is named appropriately:
> @@ -28,6 +30,7 @@ flash@0 {
>  emmc@1 {
>  	boot0-partitions {
>  		compatible = "fixed-partitions";
> +		no-fixup;
>  		#address-cells = <1>;
>  		#size-cells = <1>;

I am not so happy that we need an extra device tree property just to let
barebox know that it shouldn't try a fixup for the boot partitions.

In mci_register_partition() we unconditionally call
of_partitions_register_fixup() even for the boot partitions. Wouldn't it
be sufficient to just skip the call for the boot partitions?

Sascha

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

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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Ahmad Fatoum]

Hello Sascha,

On 7/12/18 08:43, Sascha Hauer wrote:
> On Fri, Dec 07, 2018 at 07:34:37AM +0100, Ahmad Fatoum wrote:
>> Users can have reasons to inhibit barebox from fixing up partitions nodes
>> into the kernel device tree, like if it's a boot0-partitions node, which
>> doesn't have a kernel binding.
>> Allow users to specify a no-fixup property to indicate so.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  Documentation/devicetree/bindings/mtd/partition.txt | 3 +++
>>  drivers/of/partition.c                              | 9 +++++++++
>>  fs/devfs-core.c                                     | 1 +
>>  3 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
>> index 4288a82437b4..a2e810276a9c 100644
>> --- a/Documentation/devicetree/bindings/mtd/partition.txt
>> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
>> @@ -4,6 +4,8 @@ In addition to the upstream binding, another property is added:
>>  
>>  Optional properties:
>>  - partuuid : The partition UUID for this partition.
>> +- no-fixup : empty property, indicates that barebox shouldn't fix up the
>> +             containing node into the kernel device tree.
>>  
>>  Additionally, barebox also supports partitioning the eMMC boot partitions if
>>  the partition table node is named appropriately:
>> @@ -28,6 +30,7 @@ flash@0 {
>>  emmc@1 {
>>  	boot0-partitions {
>>  		compatible = "fixed-partitions";
>> +		no-fixup;
>>  		#address-cells = <1>;
>>  		#size-cells = <1>;
> 
> I am not so happy that we need an extra device tree property just to let
> barebox know that it shouldn't try a fixup for the boot partitions.
While my use case are the emmc boot-partitions, the binding is applicable to
all fixed-partitions. Before, there has only been an universal
of_partition_binding device parameter to turn off fixups.

> 
> In mci_register_partition() we unconditionally call
> of_partitions_register_fixup() even for the boot partitions. Wouldn't it
> be sufficient to just skip the call for the boot partitions?

To expand on my reasoning, there are 3 device trees in arch/arm/dts that
use boot0-partitions. Their users would have had barebox print scary warnings
about their use on fixup unless they:
1) disabled partition fixups completely
2) added a boot0-partitions node to the kernel device tree, either unused
   or for use by custom tooling

The patch addresses both points, there is a device tree property to
selectively disable fixup and the second use case isn't broken,
unlike an unconditional (non-backward-compatible) skipping of fixups
for the boot partitions.


Thoughts?

Cheers
Ahmad

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

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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Sascha Hauer]

On Fri, Dec 07, 2018 at 09:38:37AM +0100, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 7/12/18 08:43, Sascha Hauer wrote:
> > On Fri, Dec 07, 2018 at 07:34:37AM +0100, Ahmad Fatoum wrote:
> >> Users can have reasons to inhibit barebox from fixing up partitions nodes
> >> into the kernel device tree, like if it's a boot0-partitions node, which
> >> doesn't have a kernel binding.
> >> Allow users to specify a no-fixup property to indicate so.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  Documentation/devicetree/bindings/mtd/partition.txt | 3 +++
> >>  drivers/of/partition.c                              | 9 +++++++++
> >>  fs/devfs-core.c                                     | 1 +
> >>  3 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/partition.txt b/Documentation/devicetree/bindings/mtd/partition.txt
> >> index 4288a82437b4..a2e810276a9c 100644
> >> --- a/Documentation/devicetree/bindings/mtd/partition.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/partition.txt
> >> @@ -4,6 +4,8 @@ In addition to the upstream binding, another property is added:
> >>  
> >>  Optional properties:
> >>  - partuuid : The partition UUID for this partition.
> >> +- no-fixup : empty property, indicates that barebox shouldn't fix up the
> >> +             containing node into the kernel device tree.
> >>  
> >>  Additionally, barebox also supports partitioning the eMMC boot partitions if
> >>  the partition table node is named appropriately:
> >> @@ -28,6 +30,7 @@ flash@0 {
> >>  emmc@1 {
> >>  	boot0-partitions {
> >>  		compatible = "fixed-partitions";
> >> +		no-fixup;
> >>  		#address-cells = <1>;
> >>  		#size-cells = <1>;
> > 
> > I am not so happy that we need an extra device tree property just to let
> > barebox know that it shouldn't try a fixup for the boot partitions.
> While my use case are the emmc boot-partitions, the binding is applicable to
> all fixed-partitions. Before, there has only been an universal
> of_partition_binding device parameter to turn off fixups.
> 
> > 
> > In mci_register_partition() we unconditionally call
> > of_partitions_register_fixup() even for the boot partitions. Wouldn't it
> > be sufficient to just skip the call for the boot partitions?
> 
> To expand on my reasoning, there are 3 device trees in arch/arm/dts that
> use boot0-partitions. Their users would have had barebox print scary warnings
> about their use on fixup unless they:
> 1) disabled partition fixups completely
> 2) added a boot0-partitions node to the kernel device tree, either unused
>    or for use by custom tooling
> 
> The patch addresses both points, there is a device tree property to
> selectively disable fixup and the second use case isn't broken,
> unlike an unconditional (non-backward-compatible) skipping of fixups
> for the boot partitions.

Still why do we need a device tree property when all we have to do is to
not call of_partitions_register_fixup() for the boot partitions?

sascha

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

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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Ahmad Fatoum]

Hello,

On 10/12/18 09:39, Sascha Hauer wrote:
> 
> Still why do we need a device tree property when all we have to do is to
> not call of_partitions_register_fixup() for the boot partitions?

I figured current users might want to keep the current behavior, but ok,
I will send another patch that skips fixup for the eMMC boot partitions.

You can still merge the first two patches in this patchset however, they
are applicable regardless.


Cheers
Ahmad

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

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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Ahmad Fatoum]

Hello Sascha,

On 14/12/18 16:31, Ahmad Fatoum wrote:
> You can still merge the first two patches in this patchset however, they
> are applicable regardless.

I think the first two patches slipped through the cracks. Should I resend
the series without 3/4 and 4/4?

Cheers 

> 
> 
> Cheers
> Ahmad
> 

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

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

Re: [PATCH 4/4] of: partition: add no-fixup device tree property

[Sascha Hauer]

On Thu, May 23, 2019 at 05:43:26PM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 14/12/18 16:31, Ahmad Fatoum wrote:
> > You can still merge the first two patches in this patchset however, they
> > are applicable regardless.
> 
> I think the first two patches slipped through the cracks. Should I resend
> the series without 3/4 and 4/4?

No need to resend. I applied them now.

Thanks
  Sascha

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

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