[PATCH V2] staging: vchiq_2835_arm: Make cache-line-size a required DT property

[Eric Anholt]

Stefan Wahren <stefan.wahren at i2se.com> writes:

> [add Eric]
>
>> Michael Zoran <mzoran at crowfest.net> hat am 18. Februar 2017 um 12:59 geschrieben:
>> 
>> 
>> On Sat, 2017-02-18 at 03:22 -0800, Michael Zoran wrote:
>> > The original github source allowed for the cache-line-size property
>> > to be missing.  Since recent firmwares also require this property,
>> > it makes sense to always require it in the driver as well.
>> > 
>> > If the cache-line-size property is missing, then the driver probe
>> > should fail as no dev since the kernel and dt may be out of sync.
>> > The fix is to add a check for the return value of
>> > of_property_read_u32.
>> > 
>> > Changes V2:
>> > 	1. Add error message if cache-line-size is missing.
>> > 	2. Simple check for non-zero return value from 
>> > 	   of_property_read_u32.
>> > 
>> > Signed-off-by: Michael Zoran <mzoran at crowfest.net>
>> > ---
>> >  .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c    |
>> > 8 +++++++-
>> >  1 file changed, 7 insertions(+), 1 deletion(-)
>> > 
>> > diff --git
>> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > index e6241fb5cfa6..3aeffcb9c87e 100644
>> > ---
>> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > +++
>> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
>> > @@ -121,8 +121,14 @@ int vchiq_platform_init(struct platform_device
>> > *pdev, VCHIQ_STATE_T *state)
>> >  	if (err < 0)
>> >  		return err;
>> >  
>> > -	(void)of_property_read_u32(dev->of_node, "cache-line-size",
>> > +	err = of_property_read_u32(dev->of_node, "cache-line-size",
>> >  				   &g_cache_line_size);
>> > +
>> > +	if (err) {
>> > +		dev_err(dev, "Missing cache-line-size property\n");
>> > +		return -ENODEV;
>> > +	}
>> > +
>> >  	g_fragments_size = 2 * g_cache_line_size;
>> >  
>> >  	/* Allocate space for the channels in coherent memory */
>> 
>> If anybody is willing to add an Reviewed-by, Acked, or even tested-by
>> to this it would be great.  I know gregk is a busy guy so it might make
>> sense to make a note that someone else looked at this and that I'm not
>> out in left field.
>> 
>> It would be really great if this made it into 4.11.  I'm a bit
>> concerned about someone accidently trying to mix this driver with an
>> ancient DT. Things would probably sort of work if that was done, but it
>> would result in difficult to debug errors.
>
> I think Greg is waiting for Eric's Ack, but it seems you didn't send him a copy.
>
> @Eric: Are you okay with this patch?

For staging, Greg has been taking patches without platform maintainer
ack.  I think this is great -- the staging code needs *lots* of work,
and it generally doesn't need any platform knowledge.

As far as this patch, I don't know why we wouldn't just use
cache_line_size() instead of asking DT.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rpi-kernel/attachments/20170226/7818c7c0/attachment.sig>