Re: [PATCH v2 01/12] usb: typec: add API to get port type and preferred role

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 26, 2018 at 07:49:08PM +0800, Li Jun wrote:
> This patch add 2 APIs to get port type and preferred role from firmware
> description.
> 
> Signed-off-by: Li Jun <jun.li@xxxxxxx>
> 
> ---
> change for v2
> - Change the 2 APIs name and input para to be device_node pointer.

Why?

You are only dealing with device properties here, so please move back
to using the unified device property API.

>  drivers/usb/typec/typec.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/typec.h |  2 ++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
> index 735726c..e7b2802 100644
> --- a/drivers/usb/typec/typec.c
> +++ b/drivers/usb/typec/typec.c
> @@ -9,6 +9,7 @@
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>

No need for that.

>  #include <linux/slab.h>
>  #include <linux/usb/typec.h>
>  
> @@ -1246,6 +1247,51 @@ void typec_set_pwr_opmode(struct typec_port *port,
>  }
>  EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
>  
> +/**
> + * typec_of_get_port_type - Get the typec port type
> + * @np: device node from which the property value is to be read.
> + *
> + * This routine is used by typec hardware driver to read property port type
> + * from the device firmware description.
> + *
> + * Returns typec_port_type if success, otherwise negative error code.
> + */
> +int typec_of_get_port_type(struct device_node *np)

int typec_get_port_type(struct device *dev)

I would expect you to have the function like that even if you really
were calling of_* functions in it so we would not need to change the
API later once support for ACPI or some other type of FW interface is
added. But as said, that will not be the case, and you need to use
device_property_* functions instead.

> +{
> +	const char *type_str;
> +	int ret;
> +
> +	ret = of_property_read_string(np, "port-type", &type_str);

ret = device_property_read_string(dev, "port-type", &type_str);

> +	if (ret < 0)
> +		return ret;
> +
> +	return match_string(typec_port_types, ARRAY_SIZE(typec_port_types),
> +								 type_str);
> +}
> +EXPORT_SYMBOL_GPL(typec_get_port_type);

Note that you are still exporting the old function name, but that's
fine. Just change the function name back to the original.

> +/**
> + * typec_of_get_preferred_role - Get the typec preferred role
> + * @np: device node from which the property value is to be read.
> + *
> + * This routine is used by typec hardware driver to read property default role
> + * from the device firmware description.
> + *
> + * Returns typec_role if success, otherwise negative error code.
> + */
> +int typec_of_get_preferred_role(struct device_node *np)

int typec_get_preferred_role(struct device dev)

> +{
> +	const char *power_str;
> +	int ret;
> +
> +	ret = of_property_read_string(np, "default-role", &power_str);

ret = device_property_read_string(dev, "default-role", &type_str);

> +	if (ret < 0)
> +		return ret;
> +
> +	return match_string(typec_roles, ARRAY_SIZE(typec_roles), power_str);
> +}
> +EXPORT_SYMBOL_GPL(typec_get_preferred_role);
> +
>  /* --------------------------------------- */

Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux