[v1] can: netlink: add CAN XL

[PATCH 8/9] can: calc_bittiming: add PWM calculation

Posted by Vincent Mailhol 3 months, 4 weeks ago

Perform the PWM calculation according to CiA recommendations.

Note that for databitrates greater than 5 MBPS, tqmin is less than
CAN_PWM_NS_MAX (which is defined to 200 nano seconds), consequently,
the result of the division:

  DIV_ROUND_UP(xl_ns, CAN_PWM_NS_MAX)

is one and thus the for loop automatically stops on the first
iteration giving a single PWM symbol per bit as expected. Because of
that, there is no actual need for a separate conditional branch for
when the databitrate is greater than 5 MBPS.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/dev/calc_bittiming.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/can/bittiming.h        |  7 +++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 394d6974f481..268ec6fa7c49 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -2,6 +2,7 @@
 /* Copyright (C) 2005 Marc Kleine-Budde, Pengutronix
  * Copyright (C) 2006 Andrey Volkov, Varma Electronics
  * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
+ * Copyright (C) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
  */
 
 #include <linux/units.h>
@@ -198,3 +199,38 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 		*ctrlmode |= tdc_auto;
 	}
 }
+
+int can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
+{
+	struct can_priv *priv = netdev_priv(dev);
+	const struct can_pwm_const *pwm_const = priv->xl.pwm_const;
+	struct can_pwm *pwm = &priv->xl.pwm;
+	u32 xl_tqmin = can_bit_time_tqmin(&priv->xl.data_bittiming);
+	u32 xl_ns = can_tqmin_to_ns(xl_tqmin, priv->clock.freq);
+	u32 nom_tqmin = can_bit_time_tqmin(&priv->bittiming);
+	int pwm_per_bit_max = xl_tqmin / (pwm_const->pwms_min + pwm_const->pwml_min);
+	int pwm_per_bit;
+	u32 pwm_tqmin;
+
+	/* For 5 MB/s databitrate or greater, xl_ns < CAN_PWM_NS_MAX
+	 * giving us a pwm_per_bit of 1 and the loop immediately breaks
+	 */
+	for (pwm_per_bit = DIV_ROUND_UP(xl_ns, CAN_PWM_NS_MAX);
+	     pwm_per_bit <= pwm_per_bit_max; pwm_per_bit++)
+		if (xl_tqmin % pwm_per_bit == 0)
+			break;
+
+	if (pwm_per_bit > pwm_per_bit_max) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "Can not divide the XL data phase's bit time: %u tqmin into multiple PWM symbols",
+				   xl_tqmin);
+		return -EINVAL;
+	}
+
+	pwm_tqmin = xl_tqmin / pwm_per_bit;
+	pwm->pwms = DIV_ROUND_UP_POW2(pwm_tqmin, 4);
+	pwm->pwml = pwm_tqmin - pwm->pwms;
+	pwm->pwmo = nom_tqmin % pwm_tqmin;
+
+	return 0;
+}
diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index 02cf9541a1dd..e7671a86afcc 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -180,6 +180,8 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 		   const struct can_bittiming *dbt,
 		   u32 tdc_mask, u32 *ctrlmode, u32 ctrlmode_supported);
+
+int can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack);
 #else /* !CONFIG_CAN_CALC_BITTIMING */
 static inline int
 can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
@@ -195,6 +197,11 @@ can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 	      u32 tdc_mask, u32 *ctrlmode, u32 ctrlmode_supported)
 {
 }
+
+static inline int
+can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
+{
+}
 #endif /* CONFIG_CAN_CALC_BITTIMING */
 
 void can_sjw_set_default(struct can_bittiming *bt);

-- 
2.49.1

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

Posted by kernel test robot 3 months, 4 weeks ago

Hi Vincent,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/can/dev.h:18,
                    from net/can/raw.c:53:
   include/linux/can/bittiming.h: In function 'can_calc_pwm':
>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
     204 | }
         | ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
   Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
   Selected by [y]:
   - RISCV [=y]


vim +204 include/linux/can/bittiming.h

   200	
   201	static inline int
   202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
   203	{
 > 204	}
   205	#endif /* CONFIG_CAN_CALC_BITTIMING */
   206	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

Posted by Vincent Mailhol 3 months, 4 weeks ago

On 14/10/2025 at 06:21, kernel test robot wrote:
> Hi Vincent,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
> compiler: riscv64-linux-gcc (GCC) 10.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from include/linux/can/dev.h:18,
>                     from net/can/raw.c:53:
>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>      204 | }
>          | ^
> 
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>    Selected by [y]:
>    - RISCV [=y]
> 
> 
> vim +204 include/linux/can/bittiming.h
> 
>    200	
>    201	static inline int
>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>    203	{
>  > 204	}
>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>    206	

I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.

What is surprising enough is that can_calc_tdco() has the exact same
issue but we never received a report.

I will first send a separate fix for can_calc_tdco() targeting
net/main and send the v2 for this series once the can_calc_tdco() fix
reaches net-next/main.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

Posted by Vincent Mailhol 3 months, 4 weeks ago

On 14/10/2025 at 11:05, Vincent Mailhol wrote:
> On 14/10/2025 at 06:21, kernel test robot wrote:
>> Hi Vincent,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
>> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
>> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
>> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
>> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
>> compiler: riscv64-linux-gcc (GCC) 10.5.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>    In file included from include/linux/can/dev.h:18,
>>                     from net/can/raw.c:53:
>>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>>      204 | }
>>          | ^
>>
>> Kconfig warnings: (for reference only)
>>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>>    Selected by [y]:
>>    - RISCV [=y]
>>
>>
>> vim +204 include/linux/can/bittiming.h
>>
>>    200	
>>    201	static inline int
>>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>>    203	{
>>  > 204	}
>>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>>    206	
> 
> I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.
> 
> What is surprising enough is that can_calc_tdco() has the exact same
> issue but we never received a report.

Actually, can_calc_tdco() is not getting a warning because it returns
void. But this doesn't change the fact that it should be returning an
error when CONFIG_CAN_CALC_BITTIMING is not set.

> I will first send a separate fix for can_calc_tdco() targeting
> net/main and send the v2 for this series once the can_calc_tdco() fix
> reaches net-next/main.

N.B. this issue does not impact the code when
CONFIG_CAN_CALC_BITTIMING is set. So you can continue to review while
this problem is getting addressed.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

Posted by Vincent Mailhol 3 months, 4 weeks ago

On 14/10/2025 at 11:19, Vincent Mailhol wrote:
> On 14/10/2025 at 11:05, Vincent Mailhol wrote:
>> On 14/10/2025 at 06:21, kernel test robot wrote:
>>> Hi Vincent,
>>>
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
>>> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
>>> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
>>> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
>>> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
>>> compiler: riscv64-linux-gcc (GCC) 10.5.0
>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
>>>
>>> All warnings (new ones prefixed by >>):
>>>
>>>    In file included from include/linux/can/dev.h:18,
>>>                     from net/can/raw.c:53:
>>>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>>>      204 | }
>>>          | ^
>>>
>>> Kconfig warnings: (for reference only)
>>>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>>>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>>>    Selected by [y]:
>>>    - RISCV [=y]
>>>
>>>
>>> vim +204 include/linux/can/bittiming.h
>>>
>>>    200	
>>>    201	static inline int
>>>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>>>    203	{
>>>  > 204	}
>>>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>>>    206	
>>
>> I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.
>>
>> What is surprising enough is that can_calc_tdco() has the exact same
>> issue but we never received a report.
> 
> Actually, can_calc_tdco() is not getting a warning because it returns
> void. But this doesn't change the fact that it should be returning an
> error when CONFIG_CAN_CALC_BITTIMING is not set.

I spoke too quickly. can_calc_tdco() is only called when no TDC
parameters are provided. In that case, if CONFIG_CAN_CALC_BITTIMING is
not set, doing nothing is the correct approach. i.e. there is nothing
to fix here. So there will be no fix patch to net/main and I will
directly incorporate the fix in a v2.

Sorry for the noise!


Yours sincerely,
Vincent Mailhol