Re: [PATCH V2 1/2] mtd: move code reading DT specified part probes to the core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/31/2017 12:31 PM, Boris Brezillon wrote:
On Fri, 31 Mar 2017 12:12:56 +0200
Rafał Miłecki <zajec5@xxxxxxxxx> wrote:

On 03/31/2017 11:56 AM, Boris Brezillon wrote:
On Fri, 31 Mar 2017 11:30:12 +0200
Rafał Miłecki <zajec5@xxxxxxxxx> wrote:

From: Rafał Miłecki <rafal@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux