On 6/2/22 11:54, Marc Kleine-Budde wrote:
On 02.06.2022 11:22:31, Jimmy Assarsson wrote:
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.
Ok, if you cannot tell from the USB product ID.
It should be possible, but it will eliminate the risk of me setting
wrong cfg. I don't got access to all the different devices, especially
not the old ones.
So we get:
struct kvaser_driver_info {
u32 quirks; /* KVASER_USB_HAS_ */
That holds the existing quirks and the new one.
Yep!
enum kvaser_usb_leaf_family;
const struct kvaser_usb_dev_*ops;
};
And quirks and family still affect dev->cfg.
...as is depends on sw_options read from the device?
Correct.
Best regards,
jimmy
