From: "Rafał Miłecki" <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Cc: "David Woodhouse" <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, "Brian Norris" <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Marek Vasut" <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Richard Weinberger" <richard-/L3Ra7n9ekc@public.gmane.org>, "Cyrille Pitchen" <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>, "Rob Herring" <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org>, "Frank Rowand" <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Linus Walleij" <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Rafał Miłecki" <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> Subject: Re: [PATCH V2 1/2] mtd: move code reading DT specified part probes to the core Date: Fri, 31 Mar 2017 12:46:38 +0200 [thread overview] Message-ID: <99d4944c-3eb9-5bbd-efd0-2ac6862ab1fb@gmail.com> (raw) In-Reply-To: <20170331123153.038e3ada@bbrezillon> On 03/31/2017 12:31 PM, Boris Brezillon wrote: > On Fri, 31 Mar 2017 12:12:56 +0200 > Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> On 03/31/2017 11:56 AM, Boris Brezillon wrote: >>> On Fri, 31 Mar 2017 11:30:12 +0200 >>> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >>> >>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> >>>> >>>> Handling (creating) partitions for flash devices requires using a proper >>>> driver that will read partition table (out of somewhere). We can't >>>> simply try all existing drivers one by one, so MTD subsystem allows >>>> drivers to specify a list of applicable part probes. >>>> >>>> So far physmap_of was the only driver with support for linux,part-probe >>>> DT property. Other ones had list or probes hardcoded which wasn't making >>>> them really flexible. It prevented using common flash drivers on >>>> platforms that required some specific partition table access. >>>> >>>> This commit moves support for mentioned DT property to the MTD core >>>> file. Thanks to calling it on device parse registration (as suggested by >>>> Boris) all drivers gain support for it for free. >>>> >>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> >>>> --- >>>> drivers/mtd/maps/physmap_of.c | 36 +----------------------------------- >>>> drivers/mtd/mtdcore.c | 34 ++++++++++++++++++++++++++++++++++ >>> >>> Maybe you should split the patch: >>> 1/ add core infrastructure >>> 2/ remove open-coded version in drivers/mtd/maps/physmap_of.c >> >> What's the gain of that? Is this patch too complex to review properly? Will that >> be useful for anyone to have it split? For me it only adds an intermediate code >> duplication. > > The gain is that we get rid of the dependency we have on patch "mtd: > physmap_of: use OF helpers for reading strings" which is not even > mentioned in your mails. This is definitely my mistake, initially I pushed a proper comment below tear line but have overwritten it later. Sorry! >>> BTW, not sure the intermediate "mtd: physmap_of: use OF helpers >>> for reading strings" patch is really useful, since you move to the >>> common infrastructure here. >>> By following my suggestion you get rid of the dependency you have >>> between this series and patch "mtd: physmap_of: use OF helpers for >>> reading strings". >> >> I learned (the very hard way) MTD people can be really nitpicking so I'm >> sending as simple patches as I can. I see it as the only way for someone from >> OpenWrt/LEDE project to get patch through your review. > > And I learned the hard way that OpenWRT/LEDE developers tend to not > listen to our advices and keep arguing on things that have been proven > to be existing because of bad decision they made at some point in the > project life. So I think we're even :-P. I wish you could sometimes forget what you've learned and review/discuss things without all that negative approach I keep seeing. >> It's like with this patch: even a simple code move can be questioned. Please >> drop this patchset, I'll resend it after/if I manage to get >> [PATCH] mtd: physmap_of: use OF helpers for reading strings >> accepted. > > But really, what's the point of this patch? It's just a cleanup. You're > not fixing a bug or changing the behavior, and your real objective is > to get support for the linux,part-probe in the core, which will then > allow us to drop the open-coded version you have in physmap_of.c. > > I don't think it deserves an intermediate patch, unless your real > objective is patchcount. OK, I'm going to trust that and see how easily I get can patch your way. I'll resend combined version soon. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: "Rafał Miłecki" <zajec5@gmail.com> To: Boris Brezillon <boris.brezillon@free-electrons.com> Cc: "David Woodhouse" <dwmw2@infradead.org>, "Brian Norris" <computersforpeace@gmail.com>, "Marek Vasut" <marek.vasut@gmail.com>, "Richard Weinberger" <richard@nod.at>, "Cyrille Pitchen" <cyrille.pitchen@atmel.com>, "Rob Herring" <robh+dt@kernel.org>, "Mark Rutland" <mark.rutland@arm.com>, "Frank Rowand" <frowand.list@gmail.com>, "Linus Walleij" <linus.walleij@linaro.org>, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, "Rafał Miłecki" <rafal@milecki.pl> Subject: Re: [PATCH V2 1/2] mtd: move code reading DT specified part probes to the core Date: Fri, 31 Mar 2017 12:46:38 +0200 [thread overview] Message-ID: <99d4944c-3eb9-5bbd-efd0-2ac6862ab1fb@gmail.com> (raw) In-Reply-To: <20170331123153.038e3ada@bbrezillon> On 03/31/2017 12:31 PM, Boris Brezillon wrote: > On Fri, 31 Mar 2017 12:12:56 +0200 > Rafał Miłecki <zajec5@gmail.com> wrote: > >> On 03/31/2017 11:56 AM, Boris Brezillon wrote: >>> On Fri, 31 Mar 2017 11:30:12 +0200 >>> Rafał Miłecki <zajec5@gmail.com> wrote: >>> >>>> From: Rafał Miłecki <rafal@milecki.pl> >>>> >>>> Handling (creating) partitions for flash devices requires using a proper >>>> driver that will read partition table (out of somewhere). We can't >>>> simply try all existing drivers one by one, so MTD subsystem allows >>>> drivers to specify a list of applicable part probes. >>>> >>>> So far physmap_of was the only driver with support for linux,part-probe >>>> DT property. Other ones had list or probes hardcoded which wasn't making >>>> them really flexible. It prevented using common flash drivers on >>>> platforms that required some specific partition table access. >>>> >>>> This commit moves support for mentioned DT property to the MTD core >>>> file. Thanks to calling it on device parse registration (as suggested by >>>> Boris) all drivers gain support for it for free. >>>> >>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >>>> --- >>>> drivers/mtd/maps/physmap_of.c | 36 +----------------------------------- >>>> drivers/mtd/mtdcore.c | 34 ++++++++++++++++++++++++++++++++++ >>> >>> Maybe you should split the patch: >>> 1/ add core infrastructure >>> 2/ remove open-coded version in drivers/mtd/maps/physmap_of.c >> >> What's the gain of that? Is this patch too complex to review properly? Will that >> be useful for anyone to have it split? For me it only adds an intermediate code >> duplication. > > The gain is that we get rid of the dependency we have on patch "mtd: > physmap_of: use OF helpers for reading strings" which is not even > mentioned in your mails. This is definitely my mistake, initially I pushed a proper comment below tear line but have overwritten it later. Sorry! >>> BTW, not sure the intermediate "mtd: physmap_of: use OF helpers >>> for reading strings" patch is really useful, since you move to the >>> common infrastructure here. >>> By following my suggestion you get rid of the dependency you have >>> between this series and patch "mtd: physmap_of: use OF helpers for >>> reading strings". >> >> I learned (the very hard way) MTD people can be really nitpicking so I'm >> sending as simple patches as I can. I see it as the only way for someone from >> OpenWrt/LEDE project to get patch through your review. > > And I learned the hard way that OpenWRT/LEDE developers tend to not > listen to our advices and keep arguing on things that have been proven > to be existing because of bad decision they made at some point in the > project life. So I think we're even :-P. I wish you could sometimes forget what you've learned and review/discuss things without all that negative approach I keep seeing. >> It's like with this patch: even a simple code move can be questioned. Please >> drop this patchset, I'll resend it after/if I manage to get >> [PATCH] mtd: physmap_of: use OF helpers for reading strings >> accepted. > > But really, what's the point of this patch? It's just a cleanup. You're > not fixing a bug or changing the behavior, and your real objective is > to get support for the linux,part-probe in the core, which will then > allow us to drop the open-coded version you have in physmap_of.c. > > I don't think it deserves an intermediate patch, unless your real > objective is patchcount. OK, I'm going to trust that and see how easily I get can patch your way. I'll resend combined version soon.
next prev parent reply other threads:[~2017-03-31 10:46 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-30 21:53 [PATCH 1/2] mtd: move code reading DT specified part probes to the common place Rafał Miłecki 2017-03-30 21:53 ` Rafał Miłecki [not found] ` <20170330215332.32699-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-30 21:53 ` [PATCH 2/2] dt-bindings: mtd: document linux,part-probe property Rafał Miłecki 2017-03-30 21:53 ` Rafał Miłecki 2017-03-30 23:26 ` Marek Vasut 2017-03-30 23:26 ` Marek Vasut [not found] ` <74e3a663-2431-9b86-9d90-7f2fe6ce900f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-31 5:03 ` Rafał Miłecki 2017-03-31 5:03 ` Rafał Miłecki 2017-03-31 7:42 ` [PATCH 1/2] mtd: move code reading DT specified part probes to the common place Boris Brezillon 2017-03-31 7:42 ` Boris Brezillon 2017-03-31 7:55 ` Boris Brezillon 2017-03-31 7:55 ` Boris Brezillon 2017-03-31 9:30 ` [PATCH V2 1/2] mtd: move code reading DT specified part probes to the core Rafał Miłecki 2017-03-31 9:30 ` Rafał Miłecki [not found] ` <20170331093013.29123-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-31 9:30 ` [PATCH V2 2/2] dt-bindings: mtd: document linux,part-probe property Rafał Miłecki 2017-03-31 9:30 ` Rafał Miłecki 2017-03-31 9:56 ` [PATCH V2 1/2] mtd: move code reading DT specified part probes to the core Boris Brezillon 2017-03-31 9:56 ` Boris Brezillon 2017-03-31 10:12 ` Rafał Miłecki 2017-03-31 10:12 ` Rafał Miłecki [not found] ` <40a0f980-c849-30de-c906-a708e4d90be5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-31 10:31 ` Boris Brezillon 2017-03-31 10:31 ` Boris Brezillon 2017-03-31 10:46 ` Rafał Miłecki [this message] 2017-03-31 10:46 ` Rafał Miłecki [not found] ` <99d4944c-3eb9-5bbd-efd0-2ac6862ab1fb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-31 11:41 ` Boris Brezillon 2017-03-31 11:41 ` Boris Brezillon 2017-03-31 12:23 ` Rafał Miłecki 2017-03-31 12:23 ` Rafał Miłecki [not found] ` <8d0b5b57-db3e-290e-4fa0-7ff28644ae86-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-03-31 12:27 ` Boris Brezillon 2017-03-31 12:27 ` Boris Brezillon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=99d4944c-3eb9-5bbd-efd0-2ac6862ab1fb@gmail.com \ --to=zajec5-re5jqeeqqe8avxtiumwx3w@public.gmane.org \ --cc=boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \ --cc=computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \ --cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org \ --cc=richard-/L3Ra7n9ekc@public.gmane.org \ --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.