| Message ID | 20240809172106.25892-4-ansuelsmth@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: improve block2mtd + airoha parser | expand |
On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote: > Add property for defining partitions node in mmc-card node to define > partitions in DT by the use of the block2mtd module to use block > devices as MTD. You justified patch 1 saying eMMC already supported this, but then here you add support. Both are a NAK for me as both already have a way to describe partitions with GPT. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > .../devicetree/bindings/mmc/mmc-card.yaml | 40 +++++++++++++++++++ > 1 file changed, 40 insertions(+)
On Wed, Aug 21, 2024 at 03:14:29PM +0200, Ulf Hansson wrote: > On Wed, 21 Aug 2024 at 11:52, Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote: > > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote: > > > > Add property for defining partitions node in mmc-card node to define > > > > partitions in DT by the use of the block2mtd module to use block > > > > devices as MTD. > > > > > > You justified patch 1 saying eMMC already supported this, but then here > > > you add support. > > > > > > Both are a NAK for me as both already have a way to describe partitions > > > with GPT. > > > > > > > I think this got a bit confused and hope we can find a way to add > > support for this. > > > > What is "already supported" is assigning an OF node so driver can > > reference it. This patch was just adding the nodes in the schema to say > > that partitions can be defined. > > > > I think what is not clear is that block devices might be used as raw > > devices without a partition table defined in the device. In such case > > it's the kernel that define a fixed partition table. > > > > One example is [1] where the partition table is provided by cmdline. > > Similar to cmdlinepart MTD parser. > > > > The use of block2mtd is just to make use of the MTD parser system. > > > > Considering > > - eMMC is soldered to the device (no dynamic scan) > > - cmdline might be not tunable and hardcoding it might also be > > problematic (as some cmdline needs to be used) > > - concept of fixed partition for block device is already a thing and > > used a lot (android AFAIK) > > Sorry for sidestepping your discussion, but I just wanted to add a few comments. > > It's not clear to me why we need something different than what we > already have today. It's really adding a missing feature. We have cmdline but we don't have DT. And in emebedded many times cmdline can't be changed. > > If it's a partuuid/uuid/label or a fixed block-partition from the > command line, we still need to know what partition we shall use for > what. So why is this problem different from how we manage filesystem > mounts, for example? In the context of eMMC there isn't any partition table and the cmdline is hardcoded by the bootloader. The cmdline might provide the root partition to use but doesn't declare the partition table (with cmdline) And the bootloader with the hardcoded cmdline might provide a different root partition in dual-boot-partition implementation on switching the boot partition. (this is used a lot with bootloader using a env variable and a different cmdline passed based on the variable to signal what partition to use for root) > > > > > I think it should be acceptable to introduce in DT support for defining > > fixed partition for block devices and some kind of parser system similar > > to MTD. What do you think? Would this be more acceptable? Idea is to > > just have a DT schema that makes use of the values that can be set in > > [1]. > > In DT we can describe that there is an eMMC card soldered to the > board, because it's a description of the HW. But describing what > stored inside the eMMC-flash doesn't belong in DT. > Why? This conflicts with how it's done on MTD. Also consider the fact that eMMC might contain calibration partition used for NVMEM. Describing fixed partition on a soldered eMMC that the bootloader expects at a fixed offset and won't ever change (or it will result in a brick) sounds like describing the HW to me. (it's the same principle of MTD just applied to block devices) (I know it sound strange as block devices are expected to have a partition table in MBR or GPT but reality is that it's not always the case) > > > > Hope we can find a solution to this, I'm totally OK for dropping NVMe as > > I understand it's PCIe stuff and very dynamic but OEM are making lots of > > use of eMMC and are starting to use these strange way (block2mtd) as we > > currently don't give a proper and easy solution for the task. > > I certainly appreciate that you are trying to solve the fragmentation > issue around this, but it looks like we need a different approach than > using DT to describe partitions. > Well it's either DT that can be tweaked and is part of the image or cmdline that have tons of limitation due to bootloader having fun hardcoding it or also actually requiring the bootloader cmdline and adding overlay on it. Honestly as I said adding DT support is really compleating the feature of the cmdline implementation. > > > > [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst > > > > Kind regards > Uffe
Hi Christian, ansuelsmth@gmail.com wrote on Tue, 20 Aug 2024 22:20:59 +0200: > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote: > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote: > > > Add property for defining partitions node in mmc-card node to define > > > partitions in DT by the use of the block2mtd module to use block > > > devices as MTD. > > > > You justified patch 1 saying eMMC already supported this, but then here > > you add support. > > > > Both are a NAK for me as both already have a way to describe partitions > > with GPT. > > > > I think this got a bit confused and hope we can find a way to add > support for this. > > What is "already supported" is assigning an OF node so driver can > reference it. This patch was just adding the nodes in the schema to say > that partitions can be defined. > > I think what is not clear is that block devices might be used as raw > devices without a partition table defined in the device. In such case > it's the kernel that define a fixed partition table. > > One example is [1] where the partition table is provided by cmdline. > Similar to cmdlinepart MTD parser. > > The use of block2mtd is just to make use of the MTD parser system. > > Considering > - eMMC is soldered to the device (no dynamic scan) > - cmdline might be not tunable and hardcoding it might also be > problematic (as some cmdline needs to be used) > - concept of fixed partition for block device is already a thing and > used a lot (android AFAIK) > > I think it should be acceptable to introduce in DT support for defining > fixed partition for block devices and some kind of parser system similar > to MTD. What do you think? Would this be more acceptable? Idea is to > just have a DT schema that makes use of the values that can be set in > [1]. > > Hope we can find a solution to this, I'm totally OK for dropping NVMe as > I understand it's PCIe stuff and very dynamic but OEM are making lots of > use of eMMC and are starting to use these strange way (block2mtd) as we > currently don't give a proper and easy solution for the task. Thanks for the summary. I believe I now have a better understanding of what you want to do, but I am still convinced you should not abuse the mtd layer for that. If blocks can be partitioned based on the cmdline (or from the DT) then the partitioning logic should be specific to the block layer. However, the parsing logic is probably very similar and could be extracted into a lib/, provided that the mtd bits are moved away cleverly. You only need the fixed parser anyway, and you probably don't want all the weird corner cases we keep supporting for backward compatibility reasons. > [1] https://github.com/torvalds/linux/blob/master/Documentation/block/cmdline-partition.rst > > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > --- > > > .../devicetree/bindings/mmc/mmc-card.yaml | 40 +++++++++++++++++++ > > > 1 file changed, 40 insertions(+) > Thanks, Miquèl
On Wed, 21 Aug 2024 at 20:09, Christian Marangi <ansuelsmth@gmail.com> wrote: > > On Wed, Aug 21, 2024 at 03:14:29PM +0200, Ulf Hansson wrote: > > On Wed, 21 Aug 2024 at 11:52, Christian Marangi <ansuelsmth@gmail.com> wrote: > > > > > > On Tue, Aug 13, 2024 at 02:07:34PM -0600, Rob Herring wrote: > > > > On Fri, Aug 09, 2024 at 07:21:01PM +0200, Christian Marangi wrote: > > > > > Add property for defining partitions node in mmc-card node to define > > > > > partitions in DT by the use of the block2mtd module to use block > > > > > devices as MTD. > > > > > > > > You justified patch 1 saying eMMC already supported this, but then here > > > > you add support. > > > > > > > > Both are a NAK for me as both already have a way to describe partitions > > > > with GPT. > > > > > > > > > > I think this got a bit confused and hope we can find a way to add > > > support for this. > > > > > > What is "already supported" is assigning an OF node so driver can > > > reference it. This patch was just adding the nodes in the schema to say > > > that partitions can be defined. > > > > > > I think what is not clear is that block devices might be used as raw > > > devices without a partition table defined in the device. In such case > > > it's the kernel that define a fixed partition table. > > > > > > One example is [1] where the partition table is provided by cmdline. > > > Similar to cmdlinepart MTD parser. > > > > > > The use of block2mtd is just to make use of the MTD parser system. > > > > > > Considering > > > - eMMC is soldered to the device (no dynamic scan) > > > - cmdline might be not tunable and hardcoding it might also be > > > problematic (as some cmdline needs to be used) > > > - concept of fixed partition for block device is already a thing and > > > used a lot (android AFAIK) > > > > Sorry for sidestepping your discussion, but I just wanted to add a few comments. > > > > It's not clear to me why we need something different than what we > > already have today. > > It's really adding a missing feature. We have cmdline but we don't have > DT. And in emebedded many times cmdline can't be changed. > > > > > If it's a partuuid/uuid/label or a fixed block-partition from the > > command line, we still need to know what partition we shall use for > > what. So why is this problem different from how we manage filesystem > > mounts, for example? > > In the context of eMMC there isn't any partition table and the cmdline > is hardcoded by the bootloader. The cmdline might provide the root > partition to use but doesn't declare the partition table (with cmdline) > > And the bootloader with the hardcoded cmdline might provide a different > root partition in dual-boot-partition implementation on switching the > boot partition. (this is used a lot with bootloader using a env variable > and a different cmdline passed based on the variable to signal what > partition to use for root) > > > > > > > > > I think it should be acceptable to introduce in DT support for defining > > > fixed partition for block devices and some kind of parser system similar > > > to MTD. What do you think? Would this be more acceptable? Idea is to > > > just have a DT schema that makes use of the values that can be set in > > > [1]. > > > > In DT we can describe that there is an eMMC card soldered to the > > board, because it's a description of the HW. But describing what > > stored inside the eMMC-flash doesn't belong in DT. > > > > Why? This conflicts with how it's done on MTD. Also consider the fact > that eMMC might contain calibration partition used for NVMEM. Because what is stored in the flash (eMMC) can be dynamically updated. It's not a description of the HW as such, but I guess it depends on how you see it. No matter what, you need to convince the DT maintainers that this is the way to do it. In my opinion, I think it would be better to invest our time to try a different path. > > Describing fixed partition on a soldered eMMC that the bootloader > expects at a fixed offset and won't ever change (or it will result in a > brick) sounds like describing the HW to me. (it's the same principle of > MTD just applied to block devices) I guess it's somewhat a greyzone in this kind of case, assuming you are referring to a bootloader that is stored in some ROM code that can't be updated. > > (I know it sound strange as block devices are expected to have a > partition table in MBR or GPT but reality is that it's not always the > case) I fully understand that but there are different ways to deal with that too. Maybe enforce to write an MBR/GPT when flashing, assuming it doesn't overwrite some data that is needed. Another option is to let the bootloader parse some platform specific data in the flash (unless it's hard coded in there too) and then make it update the block-devparts for the kernel command line, for example. > > > > > > > Hope we can find a solution to this, I'm totally OK for dropping NVMe as > > > I understand it's PCIe stuff and very dynamic but OEM are making lots of > > > use of eMMC and are starting to use these strange way (block2mtd) as we > > > currently don't give a proper and easy solution for the task. > > > > I certainly appreciate that you are trying to solve the fragmentation > > issue around this, but it looks like we need a different approach than > > using DT to describe partitions. > > > > Well it's either DT that can be tweaked and is part of the image or > cmdline that have tons of limitation due to bootloader having fun > hardcoding it or also actually requiring the bootloader cmdline and > adding overlay on it. Honestly as I said adding DT support is really > compleating the feature of the cmdline implementation. Another option is also to parse the platform specific data in some way and create partitions by using initramfs. Kind regards Uffe
diff --git a/Documentation/devicetree/bindings/mmc/mmc-card.yaml b/Documentation/devicetree/bindings/mmc/mmc-card.yaml index fd347126449a..0f32d24417bc 100644 --- a/Documentation/devicetree/bindings/mmc/mmc-card.yaml +++ b/Documentation/devicetree/bindings/mmc/mmc-card.yaml @@ -26,6 +26,9 @@ properties: Use this to indicate that the mmc-card has a broken hpi implementation, and that hpi should not be used. + partitions: + $ref: /schemas/mtd/partitions/partitions.yaml + required: - compatible - reg @@ -45,4 +48,41 @@ examples: }; }; + mmc1 { + #address-cells = <1>; + #size-cells = <0>; + + card@0 { + compatible = "mmc-card"; + reg = <0>; + broken-hpi; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + bootloader@0 { + label = "bootloader"; + reg = <0x00000000 0x00080000>; + }; + + tclinux@80000 { + label = "tclinux"; + reg = <0x00080000 0x02800000>; + }; + + tclinux_slave@2880000 { + label = "tclinux_slave"; + reg = <0x02880000 0x02800000>; + }; + + rootfs_data@5080000 { + label = "rootfs_data"; + reg = <0x5080000 0x00800000>; + }; + }; + }; + }; + ...
Add property for defining partitions node in mmc-card node to define partitions in DT by the use of the block2mtd module to use block devices as MTD. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- .../devicetree/bindings/mmc/mmc-card.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+)