linux-kernel - Re: [PATCH V2 1/2] dt-bindings: syscon: Add syscon-names to refer to syscon easily

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 Dec 2019 10:20:03 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Rob Herring <robh@...nel.org>
Cc:     Orson Zhai <orson.zhai@...soc.com>,
        Lee Jones <lee.jones@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        DTML <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kevin.tang@...soc.com, baolin.wang@...soc.com,
        Chunyan Zhang <chunyan.zhang@...soc.com>
Subject: Re: [PATCH V2 1/2] dt-bindings: syscon: Add syscon-names to refer to
 syscon easily

On Wed, Dec 4, 2019 at 8:26 PM Rob Herring <robh@...nel.org> wrote:
> On Wed, Dec 04, 2019 at 06:00:17PM +0100, Arnd Bergmann wrote:
> > On Wed, Dec 4, 2019 at 5:38 PM Rob Herring <robh@...nel.org> wrote:

> > I think generally speaking this would be useful for random registers that
> > logically belong to one device but are grouped with other unrelated
> > registers in a syscon, and that are in a different register offset for
> > each chip that has them. Using named properties instead of a list
> > of phandle/arg tuples with names is clearly a simpler alternative
> > and more like we do it today, but I can also see some API simplification
> > with Orson's patch without the binding getting out of hand.
>
> I understand when a phandle to a syscon is used. That's nothing new.
> What's special about Unisoc SoC that needs something new/different?
> I saw there's a large number of syscons, but I don't understand what's
> in them.
>
> If the API is this:
>
> struct regmap *syscon_regmap_lookup_by_name(struct device_node *np,
>                                        const char *name,
>                                        int arg_count, __u32 *out_args)
>
> How is 'name' being an entry in syscon-names simpler than just being the
> property name? The implementation for the latter would certainly be
> simpler.
>
> It also makes the property unparseable without knowledge outside of the
> DT (i.e. in the driver). I suppose if the number of cells for each entry
> is fixed, we could count the number of syscon-names entries to figure
> out the stride. But then if one entry needs a lot of cells, then they
> all have to have padding cells.

Good point. The syscon_regmap_lookup_by_name() interface would
work just as well when passing a property name compared to
a name listed in another property, and this would still be more in
line with what we do on other SoCs.

The only advantage I can see in having a list of phandle/arg tuples
rather than a set of properties is that it is a slightly more compact
representation in source form, but otherwise they should be equivalent
and agree about this being harder to parse in an automated way.

Orson, do you see any other reason for the combined property?
If not, could you respin the series once more with
syscon_regmap_lookup_by_name() replaced by something like:?

struct regmap *
syscon_regmap_lookup_args_by_phandle(struct device_node *np,
                                        const char *property,
                                        int arg_count, __u32 *out_args)

         Arnd