Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1195228 > unrolled thread

Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to  controller-data Mark Brown <broonie@kernel.org> - 2015-07-29 16:10 +0200
    Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data Michal Suchanek <hramrach@gmail.com> - 2015-07-30 10:30 +0200
      Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to  controller-data Mark Brown <broonie@kernel.org> - 2015-07-30 13:00 +0200

#1195228 — Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jul 29, 2015 at 12:19:57PM +0200, Michal Suchanek wrote:

Please use subject lines matching the style for the subsytsem so people
can spot that the patch is in some way relevant.

> The controller-data subnode has no compatible. This can lead to other
> drivers getting confused by it. Add a compatible to make devicetreee
> unambiguous.

I can't tell from this commit message what the issue you're trying to
fix is, sorry.  Nodes without compatible strings are entirely normal and
don't need compatible strings.  It sounds like a bug in whatever other
driver is becoming confused.

> +	if (!of_get_property(data_np, "compatible", NULL) ||
> +	    strcmp(of_get_property(data_np, "compatible", NULL),
> +		   "samsung,s3c-controller-data"))
> +		dev_err(&spi->dev, "child node 'controller-data' does not have correct compatible\n");

This will break all existing users which is not acceptable for
mainline, we need to preserve compatibility with existing device trees.

[toc] | [next] | [standalone]

#1195811 — Re: [RFC PATCH 2/2] dt: spi: s3c64xx: add compatible to controller-data

On 29 July 2015 at 20:40, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jul 29, 2015 at 08:21:34PM +0200, Michal Suchanek wrote:
>> On 29 July 2015 at 19:16, Mark Brown <broonie@kernel.org> wrote:
>
>> >> It will not break anything. It will just spam dmesg.
>
>> > I'm confused - if all this change does is to spam dmesg then what's the
>> > point?
>
>> Presumably when your SPI NOR flash fails to probe this message will be
>> just above and you will look into the binding doc and add the
>> compatible.
>
> If you're looking to add a warning message when the flash device fails
> to probe then add that in the flash driver when it detects an error,
> this will cause needless noise for everyone else using this controller
> purely to work around the broken binding.

Technically nobody needs to see the warning with in-tree boards since
the dts can be amended with the compatible.

There is no error in flash device driver.

There is only error parsing partition scheme. In my opinion this
should never cause an error. With disk drives failure to parse
partition table results in unpartitioned disk. As there are number of
partitioning schemes failure to parse one still does not prevent other
in succeding.

>
> And like I say compatible really seems like it isn't an appropriate
> property here.

So to sum up the discussion adding compatible to s3c64xx
controller-data is not desirable and making ofpart more robust is
desirable.

I think the suggestion to use a subnode for ofpart gives the most
robust solution.

Even adding compatibles to the partition subnodes ofpart still
monopolizes the address and cells property of the mtd node which does
not pass the 'if another driver did the same would it work together?'
test.

So my suggestion is to

1) search of ofpart subnode in mtd node. If present read address and
reg from it and search partitions as subnodes of the ofpart node. In
this case unknown nodes can cause error.

2) failing that issue a warning and try to parse ofpart partitions
from the mtd node itself. In this case unknown nodes cannot cause
error for compatibility with other drivers including the already
exisitn s3c64xx controller-data node.

The parser code can be the same for both cases and only operate on
different node with a flag to reject unknown subnodes or not.

Thanks

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

[toc] | [prev] | [next] | [standalone]

#1195909

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jul 30, 2015 at 10:24:37AM +0200, Michal Suchanek wrote:

> So my suggestion is to

> 1) search of ofpart subnode in mtd node. If present read address and
> reg from it and search partitions as subnodes of the ofpart node. In
> this case unknown nodes can cause error.

> 2) failing that issue a warning and try to parse ofpart partitions
> from the mtd node itself. In this case unknown nodes cannot cause
> error for compatibility with other drivers including the already
> exisitn s3c64xx controller-data node.

> The parser code can be the same for both cases and only operate on
> different node with a flag to reject unknown subnodes or not.

That seems reasonable to me.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web