subject:"\[PATCH v6 1\/6\] can\: dev\: Add support for limiting configured bitrate"

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-03 Thread Faiz Abbas

Hi,

On Tuesday 02 January 2018 09:45 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr 
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
> 
> Please add support to read the maximum bitrate via netlink. Have a look
> at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
> you need to extend the following functions: can_get_size() and
> can_fill_info().

Will add in the next version.

Thanks,
Faiz

> 
> Marc
>

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-03 Thread Faiz Abbas

Hi,

On Tuesday 02 January 2018 09:45 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr 
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
> 
> Please add support to read the maximum bitrate via netlink. Have a look
> at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
> you need to extend the following functions: can_get_size() and
> can_fill_info().

Will add in the next version.

Thanks,
Faiz

> 
> Marc
>

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-03 Thread Faiz Abbas

Hi Marc,

On Tuesday 02 January 2018 06:30 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr 
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
>>
>> Signed-off-by: Franklin S Cooper Jr 
>> [nsek...@ti.com: fix build error with !CONFIG_OF]
>> Signed-off-by: Sekhar Nori 
>> Signed-off-by: Faiz Abbas 
>> ---
>> v6 changes:
>> fix build error with !CONFIG_OF
>>
>>  drivers/net/can/dev.c   | 39 +++
>>  include/linux/can/dev.h |  8 
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>> index 365a8cc..007cfc0 100644
>> --- a/drivers/net/can/dev.c
>> +++ b/drivers/net/can/dev.c
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>>  #define MOD_DESC "CAN device driver interface"
>> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>>  }
>>  EXPORT_SYMBOL_GPL(open_candev);
>>  
>> +#ifdef CONFIG_OF
>> +/*
>> + * Common function that can be used to understand the limitation of
>> + * a transceiver when it provides no means to determine these limitations
>> + * at runtime.
>> + */
>> +void of_can_transceiver(struct net_device *dev)
>> +{
>> +struct device_node *dn;
>> +struct can_priv *priv = netdev_priv(dev);
>> +struct device_node *np = dev->dev.parent->of_node;
>> +int ret;
>> +
>> +dn = of_get_child_by_name(np, "can-transceiver");
>> +if (!dn)
>> +return;
>> +
>> +ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
>> +if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
>> +netdev_warn(dev, "Invalid value for transceiver max bitrate. 
>> Ignoring bitrate limit.\n");
>> +}
>> +EXPORT_SYMBOL_GPL(of_can_transceiver);
>> +#endif
>> +
>>  /*
>>   * Common close function for cleanup before the device gets closed.
>>   *
>> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, 
>> struct nlattr *tb[],
>>  priv->bitrate_const_cnt);
>>  if (err)
>>  return err;
>> +
>> +if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
>> +netdev_err(dev, "arbitration bitrate surpasses 
>> transceiver capabilities of %d bps\n",
>> +   priv->max_bitrate);
>> +return -EINVAL;
>> +}
> 
> What about movong this check into can_get_bittiming()? Although we loose
> the "arbitration" vs "canfd data".

I would prefer to keep the distinction.

> 
>> +
>>  memcpy(>bittiming, , sizeof(bt));
>>  
>>  if (priv->do_set_bittiming) {
>> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, 
>> struct nlattr *tb[],
>>  priv->data_bitrate_const_cnt);
>>  if (err)
>>  return err;
>> +
>> +if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
>> +netdev_err(dev, "canfd data bitrate surpasses 
>> transceiver capabilities of %d bps\n",
>> +   priv->max_bitrate);
>> +return -EINVAL;
>> +}
>> +
>>  memcpy(>data_bittiming, , sizeof(dbt));
>>  
>>  if (priv->do_set_data_bittiming) {
>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
>> index 61f1cf2..fbb7810 100644
>> --- a/include/linux/can/dev.h
>> +++ b/include/linux/can/dev.h
>> @@ -48,6 +48,8 @@ struct can_priv {
>>  unsigned int data_bitrate_const_cnt;
>>  struct can_clock clock;
>>  
>> +unsigned int max_bitrate;
> 
> Please make it an u32, name it "bitrate_max" and ...

Sure.

> 
>>> struct can_priv {
>>> struct net_device *dev;
>>> struct can_device_stats can_stats;
>>>
>>> struct can_bittiming bittiming, data_bittiming;
>>> const struct can_bittiming_const *bittiming_const,
>>> *data_bittiming_const;
>>> const u16 *termination_const;
>>> unsigned int termination_const_cnt;
>>> u16 termination;
>>> const u32 *bitrate_const;
>>> unsigned int bitrate_const_cnt;
>>> const u32 *data_bitrate_const;
>>> unsigned int data_bitrate_const_cnt;
> 
> ...move it here.

Ok.

Thanks,
Faiz

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-03 Thread Faiz Abbas

Hi Marc,

On Tuesday 02 January 2018 06:30 PM, Marc Kleine-Budde wrote:
> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>> From: Franklin S Cooper Jr 
>>
>> Various CAN or CAN-FD IP may be able to run at a faster rate than
>> what the transceiver the CAN node is connected to. This can lead to
>> unexpected errors. However, CAN transceivers typically have fixed
>> limitations and provide no means to discover these limitations at
>> runtime. Therefore, add support for a can-transceiver node that
>> can be reused by other CAN peripheral drivers to determine for both
>> CAN and CAN-FD what the max bitrate that can be used. If the user
>> tries to configure CAN to pass these maximum bitrates it will throw
>> an error.
>>
>> Signed-off-by: Franklin S Cooper Jr 
>> [nsek...@ti.com: fix build error with !CONFIG_OF]
>> Signed-off-by: Sekhar Nori 
>> Signed-off-by: Faiz Abbas 
>> ---
>> v6 changes:
>> fix build error with !CONFIG_OF
>>
>>  drivers/net/can/dev.c   | 39 +++
>>  include/linux/can/dev.h |  8 
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>> index 365a8cc..007cfc0 100644
>> --- a/drivers/net/can/dev.c
>> +++ b/drivers/net/can/dev.c
>> @@ -27,6 +27,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  
>>  #define MOD_DESC "CAN device driver interface"
>> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>>  }
>>  EXPORT_SYMBOL_GPL(open_candev);
>>  
>> +#ifdef CONFIG_OF
>> +/*
>> + * Common function that can be used to understand the limitation of
>> + * a transceiver when it provides no means to determine these limitations
>> + * at runtime.
>> + */
>> +void of_can_transceiver(struct net_device *dev)
>> +{
>> +struct device_node *dn;
>> +struct can_priv *priv = netdev_priv(dev);
>> +struct device_node *np = dev->dev.parent->of_node;
>> +int ret;
>> +
>> +dn = of_get_child_by_name(np, "can-transceiver");
>> +if (!dn)
>> +return;
>> +
>> +ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
>> +if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
>> +netdev_warn(dev, "Invalid value for transceiver max bitrate. 
>> Ignoring bitrate limit.\n");
>> +}
>> +EXPORT_SYMBOL_GPL(of_can_transceiver);
>> +#endif
>> +
>>  /*
>>   * Common close function for cleanup before the device gets closed.
>>   *
>> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, 
>> struct nlattr *tb[],
>>  priv->bitrate_const_cnt);
>>  if (err)
>>  return err;
>> +
>> +if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
>> +netdev_err(dev, "arbitration bitrate surpasses 
>> transceiver capabilities of %d bps\n",
>> +   priv->max_bitrate);
>> +return -EINVAL;
>> +}
> 
> What about movong this check into can_get_bittiming()? Although we loose
> the "arbitration" vs "canfd data".

I would prefer to keep the distinction.

> 
>> +
>>  memcpy(>bittiming, , sizeof(bt));
>>  
>>  if (priv->do_set_bittiming) {
>> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, 
>> struct nlattr *tb[],
>>  priv->data_bitrate_const_cnt);
>>  if (err)
>>  return err;
>> +
>> +if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
>> +netdev_err(dev, "canfd data bitrate surpasses 
>> transceiver capabilities of %d bps\n",
>> +   priv->max_bitrate);
>> +return -EINVAL;
>> +}
>> +
>>  memcpy(>data_bittiming, , sizeof(dbt));
>>  
>>  if (priv->do_set_data_bittiming) {
>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
>> index 61f1cf2..fbb7810 100644
>> --- a/include/linux/can/dev.h
>> +++ b/include/linux/can/dev.h
>> @@ -48,6 +48,8 @@ struct can_priv {
>>  unsigned int data_bitrate_const_cnt;
>>  struct can_clock clock;
>>  
>> +unsigned int max_bitrate;
> 
> Please make it an u32, name it "bitrate_max" and ...

Sure.

> 
>>> struct can_priv {
>>> struct net_device *dev;
>>> struct can_device_stats can_stats;
>>>
>>> struct can_bittiming bittiming, data_bittiming;
>>> const struct can_bittiming_const *bittiming_const,
>>> *data_bittiming_const;
>>> const u16 *termination_const;
>>> unsigned int termination_const_cnt;
>>> u16 termination;
>>> const u32 *bitrate_const;
>>> unsigned int bitrate_const_cnt;
>>> const u32 *data_bitrate_const;
>>> unsigned int data_bitrate_const_cnt;
> 
> ...move it here.

Ok.

Thanks,
Faiz

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-02 Thread Marc Kleine-Budde

On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr 
> 
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.

Please add support to read the maximum bitrate via netlink. Have a look
at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
you need to extend the following functions: can_get_size() and
can_fill_info().

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-02 Thread Marc Kleine-Budde

On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr 
> 
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.

Please add support to read the maximum bitrate via netlink. Have a look
at 12a6075cabc0 ("can: dev: add CAN interface termination API"). I think
you need to extend the following functions: can_get_size() and
can_fill_info().

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-02 Thread Marc Kleine-Budde

On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr 
> 
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.
> 
> Signed-off-by: Franklin S Cooper Jr 
> [nsek...@ti.com: fix build error with !CONFIG_OF]
> Signed-off-by: Sekhar Nori 
> Signed-off-by: Faiz Abbas 
> ---
> v6 changes:
> fix build error with !CONFIG_OF
> 
>  drivers/net/can/dev.c   | 39 +++
>  include/linux/can/dev.h |  8 
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 365a8cc..007cfc0 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define MOD_DESC "CAN device driver interface"
> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(open_candev);
>  
> +#ifdef CONFIG_OF
> +/*
> + * Common function that can be used to understand the limitation of
> + * a transceiver when it provides no means to determine these limitations
> + * at runtime.
> + */
> +void of_can_transceiver(struct net_device *dev)
> +{
> + struct device_node *dn;
> + struct can_priv *priv = netdev_priv(dev);
> + struct device_node *np = dev->dev.parent->of_node;
> + int ret;
> +
> + dn = of_get_child_by_name(np, "can-transceiver");
> + if (!dn)
> + return;
> +
> + ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
> + netdev_warn(dev, "Invalid value for transceiver max bitrate. 
> Ignoring bitrate limit.\n");
> +}
> +EXPORT_SYMBOL_GPL(of_can_transceiver);
> +#endif
> +
>  /*
>   * Common close function for cleanup before the device gets closed.
>   *
> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct 
> nlattr *tb[],
>   priv->bitrate_const_cnt);
>   if (err)
>   return err;
> +
> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "arbitration bitrate surpasses 
> transceiver capabilities of %d bps\n",
> +priv->max_bitrate);
> + return -EINVAL;
> + }

What about movong this check into can_get_bittiming()? Although we loose
the "arbitration" vs "canfd data".

> +
>   memcpy(>bittiming, , sizeof(bt));
>  
>   if (priv->do_set_bittiming) {
> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, 
> struct nlattr *tb[],
>   priv->data_bitrate_const_cnt);
>   if (err)
>   return err;
> +
> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "canfd data bitrate surpasses 
> transceiver capabilities of %d bps\n",
> +priv->max_bitrate);
> + return -EINVAL;
> + }
> +
>   memcpy(>data_bittiming, , sizeof(dbt));
>  
>   if (priv->do_set_data_bittiming) {
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 61f1cf2..fbb7810 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -48,6 +48,8 @@ struct can_priv {
>   unsigned int data_bitrate_const_cnt;
>   struct can_clock clock;
>  
> + unsigned int max_bitrate;

Please make it an u32, name it "bitrate_max" and ...

>> struct can_priv {
>>  struct net_device *dev;
>>  struct can_device_stats can_stats;
>> 
>>  struct can_bittiming bittiming, data_bittiming;
>>  const struct can_bittiming_const *bittiming_const,
>>  *data_bittiming_const;
>>  const u16 *termination_const;
>>  unsigned int termination_const_cnt;
>>  u16 termination;
>>  const u32 *bitrate_const;
>>  unsigned int bitrate_const_cnt;
>>  const u32 *data_bitrate_const;
>>  unsigned int data_bitrate_const_cnt;

...move it here.

>>  struct can_clock clock;
> 

> +
>   enum can_state state;
>  
>   /* CAN controller features - see include/uapi/linux/can/netlink.h */
> @@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct 
> net_device *dev,
>  unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
>

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2018-01-02 Thread Marc Kleine-Budde

On 12/22/2017 02:31 PM, Faiz Abbas wrote:
> From: Franklin S Cooper Jr 
> 
> Various CAN or CAN-FD IP may be able to run at a faster rate than
> what the transceiver the CAN node is connected to. This can lead to
> unexpected errors. However, CAN transceivers typically have fixed
> limitations and provide no means to discover these limitations at
> runtime. Therefore, add support for a can-transceiver node that
> can be reused by other CAN peripheral drivers to determine for both
> CAN and CAN-FD what the max bitrate that can be used. If the user
> tries to configure CAN to pass these maximum bitrates it will throw
> an error.
> 
> Signed-off-by: Franklin S Cooper Jr 
> [nsek...@ti.com: fix build error with !CONFIG_OF]
> Signed-off-by: Sekhar Nori 
> Signed-off-by: Faiz Abbas 
> ---
> v6 changes:
> fix build error with !CONFIG_OF
> 
>  drivers/net/can/dev.c   | 39 +++
>  include/linux/can/dev.h |  8 
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 365a8cc..007cfc0 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define MOD_DESC "CAN device driver interface"
> @@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(open_candev);
>  
> +#ifdef CONFIG_OF
> +/*
> + * Common function that can be used to understand the limitation of
> + * a transceiver when it provides no means to determine these limitations
> + * at runtime.
> + */
> +void of_can_transceiver(struct net_device *dev)
> +{
> + struct device_node *dn;
> + struct can_priv *priv = netdev_priv(dev);
> + struct device_node *np = dev->dev.parent->of_node;
> + int ret;
> +
> + dn = of_get_child_by_name(np, "can-transceiver");
> + if (!dn)
> + return;
> +
> + ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
> + if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
> + netdev_warn(dev, "Invalid value for transceiver max bitrate. 
> Ignoring bitrate limit.\n");
> +}
> +EXPORT_SYMBOL_GPL(of_can_transceiver);
> +#endif
> +
>  /*
>   * Common close function for cleanup before the device gets closed.
>   *
> @@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct 
> nlattr *tb[],
>   priv->bitrate_const_cnt);
>   if (err)
>   return err;
> +
> + if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "arbitration bitrate surpasses 
> transceiver capabilities of %d bps\n",
> +priv->max_bitrate);
> + return -EINVAL;
> + }

What about movong this check into can_get_bittiming()? Although we loose
the "arbitration" vs "canfd data".

> +
>   memcpy(>bittiming, , sizeof(bt));
>  
>   if (priv->do_set_bittiming) {
> @@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, 
> struct nlattr *tb[],
>   priv->data_bitrate_const_cnt);
>   if (err)
>   return err;
> +
> + if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
> + netdev_err(dev, "canfd data bitrate surpasses 
> transceiver capabilities of %d bps\n",
> +priv->max_bitrate);
> + return -EINVAL;
> + }
> +
>   memcpy(>data_bittiming, , sizeof(dbt));
>  
>   if (priv->do_set_data_bittiming) {
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 61f1cf2..fbb7810 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -48,6 +48,8 @@ struct can_priv {
>   unsigned int data_bitrate_const_cnt;
>   struct can_clock clock;
>  
> + unsigned int max_bitrate;

Please make it an u32, name it "bitrate_max" and ...

>> struct can_priv {
>>  struct net_device *dev;
>>  struct can_device_stats can_stats;
>> 
>>  struct can_bittiming bittiming, data_bittiming;
>>  const struct can_bittiming_const *bittiming_const,
>>  *data_bittiming_const;
>>  const u16 *termination_const;
>>  unsigned int termination_const_cnt;
>>  u16 termination;
>>  const u32 *bitrate_const;
>>  unsigned int bitrate_const_cnt;
>>  const u32 *data_bitrate_const;
>>  unsigned int data_bitrate_const_cnt;

...move it here.

>>  struct can_clock clock;
> 

> +
>   enum can_state state;
>  
>   /* CAN controller features - see include/uapi/linux/can/netlink.h */
> @@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct 
> net_device *dev,
>  unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
>  void can_free_echo_skb(struct net_device *dev, unsigned int idx);
>

[PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2017-12-22 Thread Faiz Abbas

From: Franklin S Cooper Jr 

Various CAN or CAN-FD IP may be able to run at a faster rate than
what the transceiver the CAN node is connected to. This can lead to
unexpected errors. However, CAN transceivers typically have fixed
limitations and provide no means to discover these limitations at
runtime. Therefore, add support for a can-transceiver node that
can be reused by other CAN peripheral drivers to determine for both
CAN and CAN-FD what the max bitrate that can be used. If the user
tries to configure CAN to pass these maximum bitrates it will throw
an error.

Signed-off-by: Franklin S Cooper Jr 
[nsek...@ti.com: fix build error with !CONFIG_OF]
Signed-off-by: Sekhar Nori 
Signed-off-by: Faiz Abbas 
---
v6 changes:
fix build error with !CONFIG_OF

 drivers/net/can/dev.c   | 39 +++
 include/linux/can/dev.h |  8 
 2 files changed, 47 insertions(+)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..007cfc0 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define MOD_DESC "CAN device driver interface"
@@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(open_candev);
 
+#ifdef CONFIG_OF
+/*
+ * Common function that can be used to understand the limitation of
+ * a transceiver when it provides no means to determine these limitations
+ * at runtime.
+ */
+void of_can_transceiver(struct net_device *dev)
+{
+   struct device_node *dn;
+   struct can_priv *priv = netdev_priv(dev);
+   struct device_node *np = dev->dev.parent->of_node;
+   int ret;
+
+   dn = of_get_child_by_name(np, "can-transceiver");
+   if (!dn)
+   return;
+
+   ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
+   if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
+   netdev_warn(dev, "Invalid value for transceiver max bitrate. 
Ignoring bitrate limit.\n");
+}
+EXPORT_SYMBOL_GPL(of_can_transceiver);
+#endif
+
 /*
  * Common close function for cleanup before the device gets closed.
  *
@@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct 
nlattr *tb[],
priv->bitrate_const_cnt);
if (err)
return err;
+
+   if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
+   netdev_err(dev, "arbitration bitrate surpasses 
transceiver capabilities of %d bps\n",
+  priv->max_bitrate);
+   return -EINVAL;
+   }
+
memcpy(>bittiming, , sizeof(bt));
 
if (priv->do_set_bittiming) {
@@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct 
nlattr *tb[],
priv->data_bitrate_const_cnt);
if (err)
return err;
+
+   if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
+   netdev_err(dev, "canfd data bitrate surpasses 
transceiver capabilities of %d bps\n",
+  priv->max_bitrate);
+   return -EINVAL;
+   }
+
memcpy(>data_bittiming, , sizeof(dbt));
 
if (priv->do_set_data_bittiming) {
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 61f1cf2..fbb7810 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -48,6 +48,8 @@ struct can_priv {
unsigned int data_bitrate_const_cnt;
struct can_clock clock;
 
+   unsigned int max_bitrate;
+
enum can_state state;
 
/* CAN controller features - see include/uapi/linux/can/netlink.h */
@@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct 
net_device *dev,
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
+#ifdef CONFIG_OF
+void of_can_transceiver(struct net_device *dev);
+#else
+static inline void of_can_transceiver(struct net_device *dev) { }
+#endif
+
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
 struct sk_buff *alloc_canfd_skb(struct net_device *dev,
struct canfd_frame **cfd);
-- 
2.7.4

[PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

2017-12-22 Thread Faiz Abbas

From: Franklin S Cooper Jr 

Various CAN or CAN-FD IP may be able to run at a faster rate than
what the transceiver the CAN node is connected to. This can lead to
unexpected errors. However, CAN transceivers typically have fixed
limitations and provide no means to discover these limitations at
runtime. Therefore, add support for a can-transceiver node that
can be reused by other CAN peripheral drivers to determine for both
CAN and CAN-FD what the max bitrate that can be used. If the user
tries to configure CAN to pass these maximum bitrates it will throw
an error.

Signed-off-by: Franklin S Cooper Jr 
[nsek...@ti.com: fix build error with !CONFIG_OF]
Signed-off-by: Sekhar Nori 
Signed-off-by: Faiz Abbas 
---
v6 changes:
fix build error with !CONFIG_OF

 drivers/net/can/dev.c   | 39 +++
 include/linux/can/dev.h |  8 
 2 files changed, 47 insertions(+)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..007cfc0 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define MOD_DESC "CAN device driver interface"
@@ -814,6 +815,30 @@ int open_candev(struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(open_candev);
 
+#ifdef CONFIG_OF
+/*
+ * Common function that can be used to understand the limitation of
+ * a transceiver when it provides no means to determine these limitations
+ * at runtime.
+ */
+void of_can_transceiver(struct net_device *dev)
+{
+   struct device_node *dn;
+   struct can_priv *priv = netdev_priv(dev);
+   struct device_node *np = dev->dev.parent->of_node;
+   int ret;
+
+   dn = of_get_child_by_name(np, "can-transceiver");
+   if (!dn)
+   return;
+
+   ret = of_property_read_u32(dn, "max-bitrate", >max_bitrate);
+   if ((ret && ret != -EINVAL) || (!ret && !priv->max_bitrate))
+   netdev_warn(dev, "Invalid value for transceiver max bitrate. 
Ignoring bitrate limit.\n");
+}
+EXPORT_SYMBOL_GPL(of_can_transceiver);
+#endif
+
 /*
  * Common close function for cleanup before the device gets closed.
  *
@@ -913,6 +938,13 @@ static int can_changelink(struct net_device *dev, struct 
nlattr *tb[],
priv->bitrate_const_cnt);
if (err)
return err;
+
+   if (priv->max_bitrate && bt.bitrate > priv->max_bitrate) {
+   netdev_err(dev, "arbitration bitrate surpasses 
transceiver capabilities of %d bps\n",
+  priv->max_bitrate);
+   return -EINVAL;
+   }
+
memcpy(>bittiming, , sizeof(bt));
 
if (priv->do_set_bittiming) {
@@ -997,6 +1029,13 @@ static int can_changelink(struct net_device *dev, struct 
nlattr *tb[],
priv->data_bitrate_const_cnt);
if (err)
return err;
+
+   if (priv->max_bitrate && dbt.bitrate > priv->max_bitrate) {
+   netdev_err(dev, "canfd data bitrate surpasses 
transceiver capabilities of %d bps\n",
+  priv->max_bitrate);
+   return -EINVAL;
+   }
+
memcpy(>data_bittiming, , sizeof(dbt));
 
if (priv->do_set_data_bittiming) {
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 61f1cf2..fbb7810 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -48,6 +48,8 @@ struct can_priv {
unsigned int data_bitrate_const_cnt;
struct can_clock clock;
 
+   unsigned int max_bitrate;
+
enum can_state state;
 
/* CAN controller features - see include/uapi/linux/can/netlink.h */
@@ -166,6 +168,12 @@ void can_put_echo_skb(struct sk_buff *skb, struct 
net_device *dev,
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
+#ifdef CONFIG_OF
+void of_can_transceiver(struct net_device *dev);
+#else
+static inline void of_can_transceiver(struct net_device *dev) { }
+#endif
+
 struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf);
 struct sk_buff *alloc_canfd_skb(struct net_device *dev,
struct canfd_frame **cfd);
-- 
2.7.4

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

Re: [PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

[PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

[PATCH v6 1/6] can: dev: Add support for limiting configured bitrate

10 matches

Site Navigation

Mail list logo

Footer information