linux-kernel - RE: [PATCH v9 10/10] drivers: clk: Add ZynqMP clock driver

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]
Date:   Fri, 3 Aug 2018 17:57:49 +0000
From:   Jolly Shah <JOLLYS@...inx.com>
To:     Stephen Boyd <sboyd@...nel.org>,
        "ard.biesheuvel@...aro.org" <ard.biesheuvel@...aro.org>,
        "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "matt@...eblueprint.co.uk" <matt@...eblueprint.co.uk>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "sboyd@...eaurora.org" <sboyd@...eaurora.org>,
        "sudeep.holla@....com" <sudeep.holla@....com>,
        Michal Simek <michals@...inx.com>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Tejas Patel <TEJASP@...inx.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Rajan Vaja <RAJANV@...inx.com>,
        Shubhrajyoti Datta <shubhraj@...inx.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v9 10/10] drivers: clk: Add ZynqMP clock driver

Hi Stephen,

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@...nel.org]
> Sent: Wednesday, July 25, 2018 12:49 PM
> To: ard.biesheuvel@...aro.org; dmitry.torokhov@...il.com;
> gregkh@...uxfoundation.org; hkallweit1@...il.com;
> keescook@...omium.org; linux-clk@...r.kernel.org; mark.rutland@....com;
> matt@...eblueprint.co.uk; mingo@...nel.org; mturquette@...libre.com;
> robh+dt@...nel.org; sboyd@...eaurora.org; sudeep.holla@....com; Jolly
> Shah <JOLLYS@...inx.com>; Michal Simek <michals@...inx.com>
> Cc: devicetree@...r.kernel.org; Tejas Patel <TEJASP@...inx.com>; linux-
> kernel@...r.kernel.org; Rajan Vaja <RAJANV@...inx.com>; Shubhrajyoti Datta
> <shubhraj@...inx.com>; linux-arm-kernel@...ts.infradead.org
> Subject: RE: [PATCH v9 10/10] drivers: clk: Add ZynqMP clock driver
> 
> Quoting Jolly Shah (2018-07-17 13:09:01)
> > Hi Stephen,
> >
> > Thanks for the review,
> >
> > > -----Original Message-----
> > > From: Stephen Boyd [mailto:sboyd@...nel.org]
> > > Sent: Sunday, July 08, 2018 10:27 PM
> > > To: Jolly Shah <JOLLYS@...inx.com>; ard.biesheuvel@...aro.org;
> > > dmitry.torokhov@...il.com; gregkh@...uxfoundation.org;
> > > hkallweit1@...il.com; keescook@...omium.org;
> > > linux-clk@...r.kernel.org; mark.rutland@....com;
> > > matt@...eblueprint.co.uk; Michal Simek <michals@...inx.com>;
> > > mingo@...nel.org; mturquette@...libre.com;
> > > robh+dt@...nel.org; sboyd@...eaurora.org; sudeep.holla@....com
> > > Cc: Rajan Vaja <RAJANV@...inx.com>;
> > > linux-arm-kernel@...ts.infradead.org;
> > > linux-kernel@...r.kernel.org; devicetree@...r.kernel.org; Jolly Shah
> > > <JOLLYS@...inx.com>; Tejas Patel <TEJASP@...inx.com>; Shubhrajyoti
> > > Datta <shubhraj@...inx.com>; Jolly Shah <JOLLYS@...inx.com>
> > > Subject: Re: [PATCH v9 10/10] drivers: clk: Add ZynqMP clock driver
> > >
> > > > +/**
> > > > + * zynqmp_pm_clock_get_parents() - Get the first 3 parents of
> > > > +clock for given
> > > id
> > > > + * @clock_id:  Clock ID
> > > > + * @index:     Parent index
> > > > + * @parents:   3 parents of the given clock
> > > > + *
> > > > + * This function is used to get 3 parents for the clock specified
> > > > +by
> > > > + * given clock ID.
> > > > + *
> > > > + * This API will return 3 parents with a single response. To get
> > > > + * other parents, master should call same API in loop with new
> > > > + * parent index till error is returned. E.g First call should
> > > > +have
> > > > + * index 0 which will return parents 0,1 and 2. Next call, index
> > > > + * should be 3 which will return parent 3,4 and 5 and so on.
> > > > + *
> > > > + * Return: Returns status, either success or error+reason  */
> > > > +static int zynqmp_pm_clock_get_parents(u32 clock_id, u32 index,
> > > > +u32
> > > *parents)
> > > > +{
> > > > +       struct zynqmp_pm_query_data qdata = {0};
> > > > +       u32 ret_payload[PAYLOAD_ARG_CNT];
> > >
> > > What's the endianness of this payload? Is it little endian? Or do
> > > the eemi_ops convert to CPU native endianness?
> >
> > Its little endian
> 
> Is it CPU native? This might need to be marked as __le32 for proper endianess
> code.
> 

Fixed in v11 series(posted today).

> >
> > > > +
> > > > +/**
> > > > + * zynqmp_clock_init() - Initialize zynqmp clocks
> > > > + *
> > > > + * Return: 0 on success else error code  */ static int __init
> > > > +zynqmp_clock_init(void) {
> > > > +       int ret;
> > > > +       struct device_node *np;
> > > > +
> > > > +       np = of_find_compatible_node(NULL, NULL, "xlnx,zynqmp");
> > > > +       if (!np)
> > > > +               return -ENOENT;
> > > > +       of_node_put(np);
> > > > +
> > > > +       np = of_find_compatible_node(NULL, NULL,
> > > > + "xlnx,zynqmp-clk");
> > >
> > > Why can't this be a platform device driver?
> >
> > Platform driver may probe later(an actually probing later in our case). This will
> results in clock get failure in clock consumer peripherals. So clock registration
> needs to be done earlier.
> 
> That's fine though? If a clk_get() fails because the provider isn't registered yet
> the consumer will see -EPROBE_DEFER and try again later.

You are right. Replaced init with platform driver probe in v11 series(posted today).

Thanks,
Jolly Shah