On 1/19/26 14:24, Quentin Schulz wrote:
> Hi Mikhail,
>
> On 1/19/26 1:36 AM, Mikhail Kshevetskiy wrote:
>> mtdpart internally enumerate partitions starting from zero, but
>> partition
>> driver API enumerate partitions starting from 1. Just now mtdpart does
>> not take this difference into account. This is wrong.
>>
>
> This possibly will break a few boards which may be relying on the
> wrong offset being used. Since it was wrong (according to the doc in
> include/part.h which states the partition number 1 is the first), I
> guess it's "fine" to break those?
I doubt that anything is broken. First, almost none of the boards
depends on CONFIG_MTD_PARTITIONS and CONFIG_MTD_BLOCK. Second, if
somebody really use it, he should already note a bug:
* commands like "part start <interface> <dev> <part> <varname>" will
returns values not for a queried partitions
* write/read commands will operate with a wrong partition.
>
>> Unnecessary debug message also was removed.
>>
>> Fixes: c29a6daec184 ("disk: support MTD partitions")
>> Signed-off-by: Mikhail Kshevetskiy <[email protected]>
>> ---
>> drivers/mtd/mtdpart.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
>> index 842d3e7274e..0bd4b3db022 100644
>> --- a/drivers/mtd/mtdpart.c
>> +++ b/drivers/mtd/mtdpart.c
>> @@ -1067,7 +1067,6 @@ static struct mtd_info
>> *mtd_get_partition_by_index(struct mtd_info *mtd, int ind
>> if (i++ == index)
>> return part;
>> - debug("Partition with idx=%d not found on MTD device %s\n",
>> index, mtd->name);
>> return NULL;
>> }
>> @@ -1082,9 +1081,11 @@ static int __maybe_unused
>> part_get_info_mtd(struct blk_desc *dev_desc, int part_
>> return -EINVAL;
>> }
>> - part = mtd_get_partition_by_index(master, part_idx);
>> + /* internal indexing starts from zero, but part_idx starts from
>> 1 */
>> + part = mtd_get_partition_by_index(master, part_idx - 1);
>> if (!part) {
>> - debug("Failed to find partition with idx=%d\n", part_idx);
>> + debug("Failed to find partition with idx=%d on MTD device
>> %s\n",
>> + part_idx - 1, master->name);
>
> I think it'd make more sense to patch mtd_get_partition_by_index() to
> start with i = 1 instead of i = 0, because if we ever get another user
> of this function, then we don't need to go through the same trick that
> is implemented in this patch for part_get_info_mtd(). What do you think?
Not a problem, will change it
>
> Cheers,
> Quentin
