Re: [PATCH 1/2] drivers: qcom: add command DB driver

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Wed 07 Feb 08:47 PST 2018, Lina Iyer wrote:
> On Thu, Jan 25 2018 at 20:46 +0000, Stephen Boyd wrote:
> > On 01/18, Lina Iyer wrote:
> > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
[..]
> > > +struct entry_header {
> > > +	uint64_t res_id;
> > > +	u32 priority[NUM_PRIORITY];
> > > +	u32 addr;
> > > +	u16 len;
> > > +	u16 offset;
> > 
> > Are these little endian? Needs to be __le16 and __le32 then.
> > 
> Not a device memory.

Forgot to mention this based on your other answer, but as this is not
device memory you should remove the __iomem specifier and use memremap()
instead of ioremap().

> Do we need to worry about endianness?
> 

We have done that in many of the other core drivers, I do however
suspect that Stephen was the only one who ever booted the kernel in this
mode.

[..]
> > Hopefully we never have more than one commmand db? Do consumers
> > "just know" to use this code? I haven't looked at the DT binding,
> > but perhaps consumers need to point to command db via DT phandles
> > so we can identify the consumers. That may make probe defer
> > easier too.
> > 
> There would be just one command DB for an SoC. Currently, none of the
> the clients need to probe at the time of command DB.
> 
> The producer-consumer model might help, but it is probably not needed
> here.
> 

I think it's fine to not describe this relationship in DT. We can use
probe deferral without it.

[..]
> > > +		ch[i] = id[i];
> > 
> > Is this a strcpy?
> > 
> In a way yes.
> 

I think we should use 64-bit constants throughout the kernel instead of
using strings that gets converted here...

Regards,
Bjorn