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

[Rafał Miłecki <zajec5@xxxxxxxxx>]

On Wed, 5 Feb 2020 at 18:35, Bruno Pena <brunompena@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> 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/