On 1/15/21 7:27 PM, Marc Kleine-Budde wrote:
>>> Option 4:
>>> We can introduce a struct can_bitiming_fd with the first member being the struct
>>> can_bitiming and add tdc related variables after that. This way we can use the
>>> same function to calculate the bit timing on both CAN and CAN-FD.
>>
>> While option 3 is slightly easier, my preference will go to option 4.
>
> We still need the netlink enhancement from option 3.
Just tried option 4.
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 7faf6a37d5b2..bf2326fe22a1 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -32,6 +32,10 @@ enum can_mode {
> CAN_MODE_SLEEP
> };
>
> +struct canfd_bittiming {
> + struct can_bittiming dbt;
tdc to be added here....
> +};
> +
> /*
> * CAN common private data
> */
> @@ -39,7 +43,8 @@ struct can_priv {
> struct net_device *dev;
> struct can_device_stats can_stats;
>
> - struct can_bittiming bittiming, data_bittiming;
> + struct can_bittiming bittiming;
> + struct canfd_bittiming data_bittiming;
> const struct can_bittiming_const *bittiming_const,
> *data_bittiming_const;
> const u16 *termination_const;
But I had to add that ".dbt" everywhere....
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -345,8 +345,8 @@ int open_candev(struct net_device *dev)
>
> /* For CAN FD the data bitrate has to be >= the arbitration bitrate */
> if ((priv->ctrlmode & CAN_CTRLMODE_FD) &&
> - (!priv->data_bittiming.bitrate ||
> - priv->data_bittiming.bitrate < priv->bittiming.bitrate)) {
> + (!priv->data_bittiming.dbt.bitrate ||
> + priv->data_bittiming.dbt.bitrate < priv->bittiming.bitrate)) {
> netdev_err(dev, "incorrect/missing data bit-timing\n");
> return -EINVAL;
> }
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 3ae884cdf677..c8341cbd8a66 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -239,7 +239,7 @@ static size_t can_get_size(const struct net_device *dev)
> size += nla_total_size(sizeof(u32)); /* IFLA_CAN_RESTART_MS */
> if (priv->do_get_berr_counter) /* IFLA_CAN_BERR_COUNTER */
> size += nla_total_size(sizeof(struct can_berr_counter));
> - if (priv->data_bittiming.bitrate) /* IFLA_CAN_DATA_BITTIMING */
> + if (priv->data_bittiming.dbt.bitrate) /* IFLA_CAN_DATA_BITTIMING */
> size += nla_total_size(sizeof(struct can_bittiming));
> if (priv->data_bittiming_const) /* IFLA_CAN_DATA_BITTIMING_CONST */
> size += nla_total_size(sizeof(struct can_bittiming_const));
> @@ -286,7 +286,7 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
> !priv->do_get_berr_counter(dev, &bec) &&
> nla_put(skb, IFLA_CAN_BERR_COUNTER, sizeof(bec), &bec)) ||
>
> - (priv->data_bittiming.bitrate &&
> + (priv->data_bittiming.dbt.bitrate &&
> nla_put(skb, IFLA_CAN_DATA_BITTIMING,
> sizeof(priv->data_bittiming), &priv->data_bittiming)) ||
>
And that's not even everything.
I think best it to add a "struct can_tdc" directly to the struct can_priv. Then
add a netlink interface that returns the existing can_bittiming and the can_tdc,
not as a struct, but each member with a separate tag (or whatever it's called).
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: OpenPGP digital signature
