Hi Stephen,
> -----Original Message-----
> From: Stephen Boyd [mailto:[email protected]]
> Sent: Wednesday, July 25, 2018 12:49 PM
> To: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Jolly
> Shah <[email protected]>; Michal Simek <[email protected]>
> Cc: [email protected]; Tejas Patel <[email protected]>; linux-
> [email protected]; Rajan Vaja <[email protected]>; Shubhrajyoti Datta
> <[email protected]>; [email protected]
> 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:[email protected]]
> > > Sent: Sunday, July 08, 2018 10:27 PM
> > > To: Jolly Shah <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; Michal Simek <[email protected]>;
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected]
> > > Cc: Rajan Vaja <[email protected]>;
> > > [email protected];
> > > [email protected]; [email protected]; Jolly Shah
> > > <[email protected]>; Tejas Patel <[email protected]>; Shubhrajyoti
> > > Datta <[email protected]>; Jolly Shah <[email protected]>
> > > 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
