[PATCH] mtd: core: allow mask_flags to be set for mtd_add_partition

[PATCH] mtd: core: allow mask_flags to be set for mtd_add_partition

[Bruno Pena]

This patchs makes it possible to mask certain flags for new partitions (e.g. to make them read-only).
The change consists in the addition of a new argument "mask_flags" to "mtd_add_partition" that is passed on to the "allocate_partition".

Signed-off-by: Bruno Pena <brunompena@gmail.com>
---
 drivers/mtd/mtdchar.c          | 5 ++++-
 drivers/mtd/mtdpart.c          | 3 ++-
 include/linux/mtd/partitions.h | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 0238952..0c0a6f2 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -583,7 +583,10 @@ static int mtdchar_blkpg_ioctl(struct mtd_info *mtd,
 		/* Sanitize user input */
 		p.devname[BLKPG_DEVNAMELTH - 1] = '\0';
 
-		return mtd_add_partition(mtd, p.devname, p.start, p.length);
+		/* No mtd flag masking required */
+		uint32_t mask_flags = 0;
+
+		return mtd_add_partition(mtd, p.devname, p.start, p.length, mask_flags);
 
 	case BLKPG_DEL_PARTITION:
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index b6af41b..66675a4 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -588,7 +588,7 @@ static int mtd_add_partition_attrs(struct mtd_part *new)
 }
 
 int mtd_add_partition(struct mtd_info *parent, const char *name,
-		      long long offset, long long length)
+		      long long offset, long long length, uint32_t mask_flags)
 {
 	struct mtd_partition part;
 	struct mtd_part *new;
@@ -609,6 +609,7 @@ int mtd_add_partition(struct mtd_info *parent, const char *name,
 	part.name = name;
 	part.size = length;
 	part.offset = offset;
+	part.mask_flags = mask_flags;
 
 	new = allocate_partition(parent, &part, -1, offset);
 	if (IS_ERR(new))
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 11cb0c5..1a0deeb 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -107,7 +107,7 @@ extern void deregister_mtd_parser(struct mtd_part_parser *parser);
 
 int mtd_is_partition(const struct mtd_info *mtd);
 int mtd_add_partition(struct mtd_info *master, const char *name,
-		      long long offset, long long length);
+		      long long offset, long long length, uint32_t mask_flags);
 int mtd_del_partition(struct mtd_info *master, int partno);
 uint64_t mtd_get_device_size(const struct mtd_info *mtd);
 
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: core: allow mask_flags to be set for mtd_add_partition

[Rafał Miłecki]

Hi,

On 26.11.2019 22:25, Bruno Pena wrote:
> This patchs makes it possible to mask certain flags for new partitions (e.g. to make them read-only).
> The change consists in the addition of a new argument "mask_flags" to "mtd_add_partition" that is passed on to the "allocate_partition".

Your description answers "what?" but not "why?".

This patch adds a new function argument that is never used. This seems
quite pointless.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: core: allow mask_flags to be set for mtd_add_partition

[Bruno Pena]

Hi Rafał,

A possible use case is mentioned on the description "mask certain
flags for new partitions (e.g. to make them read-only)" - I believe
this answers your "why?" question.
As for your comment about usefulness, you are very well aware this
comes from the OpenWrt pull request 2535 [1] where this new argument
is used.
The only reason why the full patch was not sent here is because it
depends on OpenWrt specific code [2] [3] that is yet to be merged on
the kernel.
For this reason - and as requested on OpenWrt - I decided to submit an
enabler patch for the kernel exported API mtd_add_partition.

[1] https://github.com/openwrt/openwrt/pull/2535
[2] https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-4.14/401-mtd-add-support-for-different-partition-parser-types.patch
[3] https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-4.19/401-mtd-add-support-for-different-partition-parser-types.patch

Best regards,
Bruno Pena


On Wed, Feb 5, 2020 at 9:31 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> Hi,
>
> On 26.11.2019 22:25, Bruno Pena wrote:
> > This patchs makes it possible to mask certain flags for new partitions (e.g. to make them read-only).
> > The change consists in the addition of a new argument "mask_flags" to "mtd_add_partition" that is passed on to the "allocate_partition".
>
> Your description answers "what?" but not "why?".
>
> This patch adds a new function argument that is never used. This seems
> quite pointless.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: core: allow mask_flags to be set for mtd_add_partition

[Rafał Miłecki]

On Wed, 5 Feb 2020 at 18:35, Bruno Pena <brunompena@gmail.com> wrote:
> A possible use case is mentioned on the description "mask certain
> flags for new partitions (e.g. to make them read-only)" - I believe
> this answers your "why?" question.
> As for your comment about usefulness, you are very well aware this
> comes from the OpenWrt pull request 2535 [1] where this new argument
> is used.
> The only reason why the full patch was not sent here is because it
> depends on OpenWrt specific code [2] [3] that is yet to be merged on
> the kernel.
> For this reason - and as requested on OpenWrt - I decided to submit an
> enabler patch for the kernel exported API mtd_add_partition.

The problem with this patch is it adds "a possible" use case and not a
real one. It describes ability to "mask certain flags" but it doesn't
explain why/when it should be needed. You most likely can't do that as
there isn't any *in-kernel* need for that.

As much as I love upstreaming OpenWrt downstream patches I think this
change should be rejected. Kernel mustn't care about providing
required API for all downstream changes of various projects. If there
is some downstream code that needs a new function argument you should
upstream both. From kernel perspective there is no need for this
change as it adds unused code.

-- 
Rafał

On Wed, 5 Feb 2020 at 18:35, Bruno Pena <brunompena@gmail.com> wrote:
>
> Hi Rafał,
>
> A possible use case is mentioned on the description "mask certain
> flags for new partitions (e.g. to make them read-only)" - I believe
> this answers your "why?" question.
> As for your comment about usefulness, you are very well aware this
> comes from the OpenWrt pull request 2535 [1] where this new argument
> is used.
> The only reason why the full patch was not sent here is because it
> depends on OpenWrt specific code [2] [3] that is yet to be merged on
> the kernel.
> For this reason - and as requested on OpenWrt - I decided to submit an
> enabler patch for the kernel exported API mtd_add_partition.
>
> [1] https://github.com/openwrt/openwrt/pull/2535
> [2] https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-4.14/401-mtd-add-support-for-different-partition-parser-types.patch
> [3] https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-4.19/401-mtd-add-support-for-different-partition-parser-types.patch
>
> Best regards,
> Bruno Pena
>
>
> On Wed, Feb 5, 2020 at 9:31 AM Rafał Miłecki <zajec5@gmail.com> wrote:
> >
> > Hi,
> >
> > On 26.11.2019 22:25, Bruno Pena wrote:
> > > This patchs makes it possible to mask certain flags for new partitions (e.g. to make them read-only).
> > > The change consists in the addition of a new argument "mask_flags" to "mtd_add_partition" that is passed on to the "allocate_partition".
> >
> > Your description answers "what?" but not "why?".
> >
> > This patch adds a new function argument that is never used. This seems
> > quite pointless.
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Rafał

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/