linux-kernel - Re: [PATCH 2/3] mtd: ofpart: do not fail probe when no partitions exist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAOMqctTThkbfv=sQ9v1i6y+ecGAfkJZ2UuNaZD1y3TJC1_ezjg@mail.gmail.com>
Date:	Tue, 28 Jul 2015 10:17:10 +0200
From:	Michal Suchanek <hramrach@...il.com>
To:	Brian Norris <computersforpeace@...il.com>
Cc:	David Woodhouse <dwmw2@...radead.org>,
	MTD Maling List <linux-mtd@...ts.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] mtd: ofpart: do not fail probe when no partitions exist

On 27 July 2015 at 22:39, Brian Norris <computersforpeace@...il.com> wrote:
> On Mon, Jul 27, 2015 at 08:30:43PM -0000, Michal Suchanek wrote:
> ...
>> The controller-data node contains no partition information and no other
>> subnodes with partition information exist.
>>
>> The ofpart code returns an error when there are subnodes of the flash DT
>> node but no partitions are found. This error is then propagated to
>> mtdpart which propagetes it to MTD probe which fails probing the flash
>> device.
>>
>> Change this condition to a warning so that flash without partitions can
>> be accessed on Exynos with ofpart support compiled in.
>
> You never replied to my suggestion here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/352206.html
>
> Particularly, "just define a proper compatibile property for [the
> 'controller-data'] subnode, and ofpart.c will naturally handle this".
>
>> Signed-off-by: Michal Suchanek <hramrach@...il.com>
>>
>> --
>>  - add more verbose explanation
>> ---
>>  drivers/mtd/ofpart.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
>> index aa26c32..a29d29f 100644
>> --- a/drivers/mtd/ofpart.c
>> +++ b/drivers/mtd/ofpart.c
>> @@ -94,10 +94,10 @@ static int parse_ofpart_partitions(struct mtd_info *master,
>>
>>       if (!i) {
>>               of_node_put(pp);
>> -             pr_err("No valid partition found on %s\n", node->full_name);
>> +             pr_warn("No valid partition found on %s\n", node->full_name);
>>               kfree(*pparts);
>>               *pparts = NULL;
>> -             return -EINVAL;
>> +             return 0;
>
> I don't really like this, since it can turn other invalid device trees
> into a silent fallback. I'd really prefer we make it easy to tell the
> difference between a MTD partition subnode and another foo-bar subnode.

Ok, so I don't find it reasonable to have one driver handle devicetree
nodes without compatible property and insist that no other driver ever
defines nodes without a compatible property.

So either drivers that parse nodes without a compatible property
should handle nodes that are meant to be used by other drivers or
*every* driver has to use a compatible property including ofpart and
then no collision can happen.

So this goes both ways - either deal with nodes that have no
compatible but are not for your driver or define a compatible for your
driver. The s3c64xx driver is ahead of ofpart here since it just tries
to read a property from a subnode of particular name and falls back to
default if not present.

All in all there is no 'foo-bar devicetree'. The fact that your driver
does not understand a particular part of a devicetree does not mean
the part is incorrect. You cannot know what drivers will emerge in the
future and what bindings will they use. In fact this very issue shows
that you make wrong assumptions about that.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/