On 6/2/22 10:02, Marc Kleine-Budde wrote:
On 02.06.2022 08:30:30, Jimmy Assarsson wrote:
The firmware of M32C based Leaf devices expects bittiming parameters
calculated for 16MHz clock.
Since we use the actual clock frequency of the device, the device may end
up with wrong bittiming parameters, depending on user requested parameters.
This regression affects M32C based Leaf devices with non-16MHz clock.
Oh. Thanks for the patch!
Fixes: fb12797ab1fe ("can: kvaser_usb: get CAN clock frequency from device")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Jimmy Assarsson <extja@xxxxxxxxxx>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 4 +++
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 20 +++++++++++----
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 25 ++++++++++++-------
3 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index 3a49257f9fa6..cb588228d7a1 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -44,6 +44,9 @@
#define KVASER_USB_CAP_EXT_CAP 0x02
#define KVASER_USB_HYDRA_CAP_EXT_CMD 0x04
+/* Quriks */
^^^^^^
Typo
Oops!
+#define KVASER_USB_QUIRK_IGNORE_CLK_FREQ BIT(0)
+
struct kvaser_usb_dev_cfg;
enum kvaser_usb_leaf_family {
@@ -65,6 +68,7 @@ struct kvaser_usb_dev_card_data_hydra {
struct kvaser_usb_dev_card_data {
u32 ctrlmode_supported;
u32 capabilities;
+ u32 quirks;
union {
struct {
enum kvaser_usb_leaf_family family;
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index e67658b53d02..5880e9719c9d 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -94,10 +94,14 @@
static inline bool kvaser_is_leaf(const struct usb_device_id *id)
{
- return (id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
- id->idProduct <= USB_CAN_R_PRODUCT_ID) ||
- (id->idProduct >= USB_LEAF_LITE_V2_PRODUCT_ID &&
- id->idProduct <= USB_LEAF_PRODUCT_ID_END);
+ return id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
+ id->idProduct <= USB_CAN_R_PRODUCT_ID;
+}
+
+static inline bool kvaser_is_leafimx(const struct usb_device_id *id)
+{
+ return id->idProduct >= USB_LEAF_LITE_V2_PRODUCT_ID &&
+ id->idProduct <= USB_LEAF_PRODUCT_ID_END;
}
Is this getting a bit complicated now?
In this driver we have:
1) struct usb_device_id::driver_info
2) kvaser_is_*()
which is used to set
3) dev->card_data.leaf.family
4) dev->ops
and now you're adding:
5) dev->card_data.quirks
which then affects
6) dev->cfg
The straight forward way would be to define a struct that describes the
a device completely:
struct kvaser_driver_info {
u32 quirks; /* KVASER_USB_HAS_ */
enum kvaser_usb_leaf_family;
const struct kvaser_usb_dev_*ops;
const struct kvaser_usb_dev_*cfg;
};
and then assign that to every device listed in the kvaser_usb_table.
Thanks for the feedback!
I agree, but I prefer if we can keep assigning dev->cfg based on the
information that we get from the device.
So we get:
struct kvaser_driver_info {
u32 quirks; /* KVASER_USB_HAS_ */
enum kvaser_usb_leaf_family;
const struct kvaser_usb_dev_*ops;
};
And quirks and family still affect dev->cfg.
Do you think this is OK?
Best regards,
jimmy
Marc
