Re: [PATCH,RFC] usb: add devicetree helpers for determining dr_mode and phy_type

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Hi,

On Tue, Jan 29, 2013 at 07:40:23PM +0530, kishon wrote:
> On Tuesday 29 January 2013 07:23 PM, Wolfram Sang wrote:
> >>>+	err = of_property_read_string(np, "phy_type", &phy_type);
> >>>+	if (err < 0)
> >>>+		return USBPHY_INTERFACE_MODE_NA;
> >>
> >>Why don't we use a u32 property type for the *phy-type*? IMHO we
> >>should use string property only when the property should be
> >>absolutely unambiguous (e.g., compatible property should be string).
> >
> >If we would use u32-numbers in the compatible entry, this would also be
> >unambiguous, no? 0xd00dfeed would be the at24-driver. Pretty specific.
> 
> hehe... But we don't have a corresponding *enum* representing the
> drivers :-)
> >
> >I don't mind having readable devicetrees. And we have it for ethernet
> >phys already with strings, so it would be consistent.
> 
> Ok. Fine with it then :-)

I prefer u32 here, because we have the matching enum. Otherwise we end
up with:

of_property_read_string(...,&type);

if (!strcmp(type, "ulpi"))
	foo();
else if (!strcmp(type, "utmi"))
	bar();
else if (!strcmp(type, "pipe3"))
	baz();
else
	BUG();

and I don't like that, it's ugly and error prone. Also looks awful when
someone needs to change that, the diff looks messy. A u32 followed by a
switch statement looks nicer.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]