Re: [PATCH v2] can: pcan_usb: don't provide CAN FD bittimings by non-FD adapters

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On 08/06/2015 11:24 AM, Oliver Hartkopp wrote:
> Hi Marc,
> 
> On 06.08.2015 10:07, Marc Kleine-Budde wrote:
>> The CAN FD data bittiming constants are provided via netlink only when there
>> are valid CAN FD constants available in priv->data_bittiming_const.
>>
>> Due to the indirection of pointer assignments in the peak_usb driver the
>> priv->data_bittiming_const never becomes NULL - not even for non-FD adapters.
>>
>> The data_bittiming_const points to zero'ed data which leads to this result
>> when running 'ip -details link show can0':
>>
>> 35: can0: <NOARP,ECHO> mtu 16 qdisc noop state DOWN mode DEFAULT group default qlen 10
>>      link/can  promiscuity 0
>>      can state STOPPED restart-ms 0
>> 	  pcan_usb: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
>> 	  : dtseg1 0..0 dtseg2 0..0 dsjw 1..0 dbrp 0..0 dbrp-inc 0  <== BROKEN!
>> 	  clock 8000000
>>
>> This patch sets the dev_set_data_bittiming in struct peak_adapter to NULL to
>> be able to disable the CAN FD specific information on non-FD adapters.
>>
>> This patch changes the struct peak_usb_adapter::bittiming_const and struct
>> peak_usb_adapter::data_bittiming_const to pointers to fix the assignemnt
>> problems.
> 
> I assumed there is an even better solution for the issue.
> Thanks for the rework!
> 
>>
>> Cc: linux-stable <stable@vger.kernel.org> # > 4.0
> 
> Should be:
> 
> Cc: linux-stable <stable@vger.kernel.org> # >= 4.0
> 
>> Reported-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> At least with PCAN USB and PCAN USB FD - my 'pro' adapters are at the office.
> But I can see no reason why they should not work in the same manner.
> 
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Hello,
>>
>> picking up Olvier's work. Alternate approach, I've made the
>> peak_usb_adapter::bittiming_const and struct
>> peak_usb_adapter::data_bittiming_const a pointer.
>>
>> Please test,
> 
> done :-)

Thanks.
Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]