lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABXOdTdz-4HYogfMi2vvjjxk-NmSAh9YZgrMNDB-o0Zma-b5Jw@mail.gmail.com>
Date:	Tue, 26 Jul 2016 15:06:55 -0700
From:	Guenter Roeck <groeck@...gle.com>
To:	Chanwoo Choi <cw00.choi@...sung.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	myungjoo.ham@...sung.com, Chris Zhong <zyw@...k-chips.com>,
	Guenter Roeck <groeck@...omium.org>, cwchoi00@...il.com
Subject: Re: [PATCH 2/6] extcon: Add the support for extcon property according
 to extcon type

On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi <cw00.choi@...sung.com> wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
>                         unsigned int prop,
>                         union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
>                         unsigned int prop,
>                         union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> ---
>  drivers/extcon/extcon.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/extcon.h  |  86 ++++++++++++++++++++
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
>         struct device_attribute attr_state;
>
>         struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +       union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
> +       union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
> +       union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
> +       union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev *edev, const unsigned int id
>         return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +       switch (prop) {
> +       case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +               return EXTCON_TYPE_USB;
> +       case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +               return EXTCON_TYPE_CHG;
> +       case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +               return EXTCON_TYPE_JACK;
> +       case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +               return EXTCON_TYPE_DISP;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
> +                               unsigned int index)
> +{
> +       return ((!!(edev->state & (1 << index))) == 1) ? true : false;
> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>  {
>         if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>         return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
> +{
> +       unsigned int type;
> +

int

> +       /* Check whether the property is supported or not. */
> +       type = get_extcon_type(prop);
> +       if (type < 0)

otherwise type is never < 0

> +               return false;
> +
> +       /* Check whether a specific extcon id supports the property or not. */
> +       if (extcon_info[id].type | type)

This is always true ?

> +               return true;
> +
> +       return false;
> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type)                          \
> +       if (EXTCON_TYPE_##name || type) {                               \

This is always true ?

> +               for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)          \
> +                       cable->name_lower##_propval[i] = val;           \
> +       }                                                               \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int id, int index)
> +{
> +       unsigned int type = extcon_info[id].type;
> +       struct extcon_cable *cable = &edev->cables[index];
> +       union extcon_property_value val = (union extcon_property_value)(0);
> +       int i;
> +
> +       INIT_PROPERTY(USB, usb, type);
> +       INIT_PROPERTY(CHG, chg, type);
> +       INIT_PROPERTY(JACK, jack, type);
> +       INIT_PROPERTY(DISP, disp, type);
> +}
> +
>  static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>                           char *buf)
>  {
> @@ -421,7 +483,7 @@ int extcon_get_cable_state_(struct extcon_dev *edev, const unsigned int id)
>         if (edev->max_supported && edev->max_supported <= index)
>                 return -EINVAL;
>
> -       return !!(edev->state & (1 << index));
> +       return (int)(is_extcon_attached(edev, id, index));
>  }
>  EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
>
> @@ -449,12 +511,154 @@ int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id,
>         if (edev->max_supported && edev->max_supported <= index)
>                 return -EINVAL;
>
> +       /*
> +        * Initialize the value of extcon property before setting
> +        * the detached state for an external connector.
> +        */
> +       if (!cable_state)
> +               init_property(edev, id, index);
> +
> +       /* Set the state for external connector as the detached state. */
>         state = cable_state ? (1 << index) : 0;
>         return extcon_update_state(edev, 1 << index, state);
>  }
>  EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
>
>  /**
> + * extcon_get_property() - Get the property value of a specific cable.
> + * @edev:              the extcon device that has the cable.
> + * @id:                        the unique id of each external connector
> + *                     in extcon enumeration.
> + * @prop:              the property id among enum extcon_property.
> + * @prop_val:          the pointer which store the value of property.
> + *
> + * When getting the property value of external connector, the external connector
> + * should be attached. If detached state, function just return 0 without
> + * property value. Also, the each property should be included in the list of
> + * supported properties according to the type of external connectors.
> + *
> + * Returns 0 if success or error number if fail
> + */
> +int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> +                               unsigned int prop,
> +                               union extcon_property_value *prop_val)
> +{
> +       struct extcon_cable *cable;
> +       unsigned long flags;
> +       int index, ret = 0;
> +
> +       *prop_val = (union extcon_property_value)(0);
> +
> +       if (!edev)
> +               return -EINVAL;
> +
> +       /* Check whether the property is supported or not */
> +       if (!is_extcon_property_supported(id, prop))
> +               return -EINVAL;
> +
> +       /* Find the cable index of external connector by using id */
> +       index = find_cable_index_by_id(edev, id);
> +       if (index < 0)
> +               return index;
> +
> +       /*
> +        * Check whether the external connector is attached.
> +        * If external connector is detached, the user can not
> +        * get the property value.
> +        */
> +       if (!is_extcon_attached(edev, id, index))
> +               return 0;
> +
> +       cable = &edev->cables[index];
> +       spin_lock_irqsave(&edev->lock, flags);
> +
> +       /* Get the property value according to extcon type */
> +       switch (prop) {
> +       case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +               *prop_val = cable->usb_propval[prop - EXTCON_PROP_USB_MIN];
> +               break;
> +       case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +               *prop_val = cable->chg_propval[prop - EXTCON_PROP_CHG_MIN];
> +               break;
> +       case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +               *prop_val = cable->jack_propval[prop - EXTCON_PROP_JACK_MIN];
> +               break;
> +       case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +               *prop_val = cable->disp_propval[prop - EXTCON_PROP_DISP_MIN];
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       }
> +
> +       spin_unlock_irqrestore(&edev->lock, flags);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(extcon_get_property);
> +
> +/**
> + * extcon_set_property() - Set the property value of a specific cable.
> + * @edev:              the extcon device that has the cable.
> + * @id:                        the unique id of each external connector
> + *                     in extcon enumeration.
> + * @prop:              the property id among enum extcon_property.
> + * @prop_val:          the pointer including the new value of property.
> + *
> + * The each property should be included in the list of supported properties
> + * according to the type of external connectors.
> + *
> + * Returns 0 if success or error number if fail
> + */
> +int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> +                               unsigned int prop,
> +                               union extcon_property_value prop_val)
> +{
> +       struct extcon_cable *cable;
> +       unsigned long flags;
> +       int index, ret = 0;
> +
> +       if (!edev)
> +               return -EINVAL;
> +
> +       /* Check whether the property is supported or not */
> +       if (!is_extcon_property_supported(id, prop))
> +               return -EINVAL;
> +
> +       /* Find the cable index of external connector by using id */
> +       index = find_cable_index_by_id(edev, id);
> +       if (index < 0)
> +               return index;
> +
> +       cable = &edev->cables[index];
> +       spin_lock_irqsave(&edev->lock, flags);
> +
> +       /* Set the property value according to extcon type */
> +       switch (prop) {
> +       case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +               cable->usb_propval[prop - EXTCON_PROP_USB_MIN] = prop_val;
> +               break;
> +       case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +               cable->chg_propval[prop - EXTCON_PROP_CHG_MIN] = prop_val;
> +               break;
> +       case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +               cable->jack_propval[prop - EXTCON_PROP_JACK_MIN] = prop_val;
> +               break;
> +       case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +               cable->disp_propval[prop - EXTCON_PROP_DISP_MIN] = prop_val;
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       }
> +
> +       spin_unlock_irqrestore(&edev->lock, flags);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(extcon_set_property);
> +
> +/**
>   * extcon_get_extcon_dev() - Get the extcon device instance from the name
>   * @extcon_name:       The extcon name provided with extcon_dev_register()
>   */
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index 46d802892c82..296d1452dcb4 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -77,6 +77,68 @@
>
>  #define EXTCON_NUM             63
>
> +/*
> + * Define the property of supported external connectors.
> + *
> + * When adding the new extcon property, they *must* have
> + * the type/value/default information. Also, you *have to*
> + * modify the EXTCON_PROP_[type]_START/END definitions
> + * which mean the range of the supported properties
> + * for each extcon type.
> + *
> + * The naming style of property
> + * : EXTCON_PROP_[type]_[property name]
> + *
> + * EXTCON_PROP_USB_[property name]     : USB property
> + * EXTCON_PROP_CHG_[property name]     : Charger property
> + * EXTCON_PROP_JACK_[property name]    : Jack property
> + * EXTCON_PROP_DISP_[property name]    : Display property
> + */
> +
> +/*
> + * Properties of EXTCON_TYPE_USB.
> + *
> + * - EXTCON_PROP_USB_ID
> + * @type:      integer (intval)
> + * @value:     0 (low) or 1 (high)
> + * @default:   0 (low)
> + * - EXTCON_PROP_USB_VBUS
> + * @type:      integer (intval)
> + * @value:     0 (low) or 1 (high)
> + * @default:   0 (low)
> + */
> +#define EXTCON_PROP_USB_ID             0
> +#define EXTCON_PROP_USB_VBUS           1
> +
> +#define EXTCON_PROP_USB_MIN            0
> +#define EXTCON_PROP_USB_MAX            1
> +#define EXTCON_PROP_USB_CNT    (EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN)

        EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1

Otherwise the array won't have enough entries, and writing the last
property will end up overwriting usb_bits (because all other arrays
have a size of 0).

> +
> +/* Properties of EXTCON_TYPE_CHG. */
> +#define EXTCON_PROP_CHG_MIN            50
> +#define EXTCON_PROP_CHG_MAX            50
> +#define EXTCON_PROP_CHG_CNT    (EXTCON_PROP_CHG_MAX - EXTCON_PROP_CHG_MIN)

        EXTCON_PROP_CHG_MAX - EXTCON_PROP_CHG_MIN + 1

Otherwise the array won't have any entries.

> +/* Properties of EXTCON_TYPE_JACK. */
> +#define EXTCON_PROP_JACK_MIN           100
> +#define EXTCON_PROP_JACK_MAX           100
> +#define EXTCON_PROP_JACK_CNT   (EXTCON_PROP_JACK_MAX - EXTCON_PROP_JACK_MIN)


             + 1

> +
> +/* Properties of EXTCON_TYPE_DISP. */
> +#define EXTCON_PROP_DISP_MIN           150
> +#define EXTCON_PROP_DISP_MAX           150
> +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX - EXTCON_PROP_DISP_MIN)

             + 1

> +
> +/*
> + * Define the type of property's value.
> + *
> + * Define the property's value as union type. Because each property
> + * would need the different data type to store it.
> + */
> +union extcon_property_value {
> +       int intval;     /* type : interger (intval) */
> +};
> +
>  struct extcon_cable;
>
>  /**
> @@ -167,6 +229,17 @@ extern int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id,
>                                    bool cable_state);
>
>  /*
> + * get/set_property access the property value of each external connector.
> + * They are used to access the property of each cable based on the property id.
> + */
> +extern int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> +                               unsigned int prop,
> +                               union extcon_property_value *prop_val);
> +extern int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> +                               unsigned int prop,
> +                               union extcon_property_value prop_val);
> +
> +/*
>   * Following APIs are to monitor every action of a notifier.
>   * Registrar gets notified for every external port of a connection device.
>   * Probably this could be used to debug an action of notifier; however,
> @@ -239,6 +312,19 @@ static inline int extcon_set_cable_state_(struct extcon_dev *edev,
>         return 0;
>  }
>
> +static inline int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> +                                       unsigned int prop,
> +                                       union extcon_property_value *prop_val)
> +{
> +       return 0;
> +}
> +static inline int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> +                                       unsigned int prop,
> +                                       union extcon_property_value prop_val)
> +{
> +       return 0;
> +}
> +
>  static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
>  {
>         return NULL;
> --
> 1.9.1
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ