* [PATCH 0/3] can/peak_usb: Added improvement to the peak_usb driver
@ 2021-03-09 8:21 Stephane Grosjean
2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Stephane Grosjean @ 2021-03-09 8:21 UTC (permalink / raw)
To: linux-can Mailing List; +Cc: Stephane Grosjean
This series of patches adds the following improvements:
- allows to identify CAN ports by their LEDs
- completes the list of devices supported by the module
- adds ONE_SHOT mode to some devices
Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Stephane Grosjean (3):
can/peak_usb: add support of ethtool set_phys_id() callback
can/peak_usb: add forgotten supported devices
can/peak_usb: add support of ONE_SHOT mode
drivers/net/can/usb/peak_usb/pcan_usb.c | 47 ++++++++++++++++++++
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 ++
drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 +
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 47 ++++++++++++++++++--
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 46 +++++++++++++++++--
drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 6 +++
6 files changed, 144 insertions(+), 8 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() 2021-03-09 8:21 [PATCH 0/3] can/peak_usb: Added improvement to the peak_usb driver Stephane Grosjean @ 2021-03-09 8:21 ` Stephane Grosjean 2021-03-09 11:20 ` Marc Kleine-Budde 2021-03-09 12:15 ` Marc Kleine-Budde 2021-03-09 8:21 ` [PATCH 2/3] can/peak_usb: add forgotten supported devices Stephane Grosjean 2021-03-09 8:21 ` [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode Stephane Grosjean 2 siblings, 2 replies; 24+ messages in thread From: Stephane Grosjean @ 2021-03-09 8:21 UTC (permalink / raw) To: linux-can Mailing List; +Cc: Stephane Grosjean This patch makes it possible to specifically flash the LED of a CAN port of the CAN-USB interfaces of PEAK-System. Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> --- drivers/net/can/usb/peak_usb/pcan_usb.c | 47 ++++++++++++++++++++ drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 ++ drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 + drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 33 ++++++++++++++ drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 34 +++++++++++++- drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 6 +++ 6 files changed, 125 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c index e6c1e5d33924..aa89c1119e27 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c @@ -11,6 +11,7 @@ #include <linux/netdevice.h> #include <linux/usb.h> #include <linux/module.h> +#include <linux/ethtool.h> #include <linux/can.h> #include <linux/can/dev.h> @@ -42,6 +43,7 @@ MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB adapter"); #define PCAN_USB_CMD_REGISTER 9 #define PCAN_USB_CMD_EXT_VCC 10 #define PCAN_USB_CMD_ERR_FR 11 +#define PCAN_USB_CMD_LED 12 /* PCAN_USB_CMD_SET_BUS number arg */ #define PCAN_USB_BUS_XCVER 2 @@ -250,6 +252,15 @@ static int pcan_usb_set_ext_vcc(struct peak_usb_device *dev, u8 onoff) return pcan_usb_send_cmd(dev, PCAN_USB_CMD_EXT_VCC, PCAN_USB_SET, args); } +static int pcan_usb_set_led(struct peak_usb_device *dev, u8 onoff) +{ + u8 args[PCAN_USB_CMD_ARGS_LEN] = { + [0] = !!onoff, + }; + + return pcan_usb_send_cmd(dev, PCAN_USB_CMD_LED, PCAN_USB_SET, args); +} + /* * set bittiming value to can */ @@ -973,6 +984,40 @@ static int pcan_usb_probe(struct usb_interface *intf) return 0; } +static int pcan_usb_set_phys_id(struct net_device *netdev, + enum ethtool_phys_id_state state) +{ + struct peak_usb_device *dev = netdev_priv(netdev); + int err = 0; + + switch (state) { + case ETHTOOL_ID_ACTIVE: + /* call ON/OFF twice a second */ + return 2; + + case ETHTOOL_ID_OFF: + err = pcan_usb_set_led(dev, 0); + break; + + case ETHTOOL_ID_ON: + fallthrough; + + case ETHTOOL_ID_INACTIVE: + /* restore LED default */ + err = pcan_usb_set_led(dev, 1); + break; + + default: + break; + } + + return err; +} + +static const struct ethtool_ops pcan_usb_ethtool_ops = { + .set_phys_id = pcan_usb_set_phys_id, +}; + /* * describe the PCAN-USB adapter */ @@ -1003,6 +1048,8 @@ const struct peak_usb_adapter pcan_usb = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb), + .ethtool_ops = &pcan_usb_ethtool_ops, + /* timestamps usage */ .ts_used_bits = 16, .ts_period = 24575, /* calibration period in ts. */ diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c index 573b11559d73..685c59e2cfd3 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c @@ -14,6 +14,7 @@ #include <linux/module.h> #include <linux/netdevice.h> #include <linux/usb.h> +#include <linux/ethtool.h> #include <linux/can.h> #include <linux/can/dev.h> @@ -820,6 +821,9 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter, netdev->flags |= IFF_ECHO; /* we support local echo */ + /* add ethtool support */ + netdev->ethtool_ops = peak_usb_adapter->ethtool_ops; + init_usb_anchor(&dev->rx_submitted); init_usb_anchor(&dev->tx_submitted); diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h index 4b1528a42a7b..e15b4c78f309 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h @@ -46,6 +46,8 @@ struct peak_usb_adapter { const struct can_bittiming_const * const data_bittiming_const; unsigned int ctrl_count; + const struct ethtool_ops *ethtool_ops; + int (*intf_probe)(struct usb_interface *intf); int (*dev_init)(struct peak_usb_device *dev); diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index f347ecc79aef..6183a42f6491 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -7,6 +7,7 @@ #include <linux/netdevice.h> #include <linux/usb.h> #include <linux/module.h> +#include <linux/ethtool.h> #include <linux/can.h> #include <linux/can/dev.h> @@ -1009,6 +1010,31 @@ static void pcan_usb_fd_free(struct peak_usb_device *dev) } } +/* blink LED's */ +static int pcan_usb_fd_set_phys_id(struct net_device *netdev, + enum ethtool_phys_id_state state) +{ + struct peak_usb_device *dev = netdev_priv(netdev); + int err = 0; + + switch (state) { + case ETHTOOL_ID_ACTIVE: + err = pcan_usb_fd_set_can_led(dev, PCAN_UFD_LED_FAST); + break; + case ETHTOOL_ID_INACTIVE: + err = pcan_usb_fd_set_can_led(dev, PCAN_UFD_LED_DEF); + break; + default: + break; + } + + return err; +} + +static const struct ethtool_ops pcan_usb_fd_ethtool_ops = { + .set_phys_id = pcan_usb_fd_set_phys_id, +}; + /* describes the PCAN-USB FD adapter */ static const struct can_bittiming_const pcan_usb_fd_const = { .name = "pcan_usb_fd", @@ -1050,6 +1076,8 @@ const struct peak_usb_adapter pcan_usb_fd = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), + .ethtool_ops = &pcan_usb_fd_ethtool_ops, + /* timestamps usage */ .ts_used_bits = 32, .ts_period = 1000000, /* calibration period in ts. */ @@ -1122,6 +1150,7 @@ const struct peak_usb_adapter pcan_usb_chip = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), + .ethtool_ops = &pcan_usb_fd_ethtool_ops, /* timestamps usage */ .ts_used_bits = 32, @@ -1196,6 +1225,8 @@ const struct peak_usb_adapter pcan_usb_pro_fd = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), + .ethtool_ops = &pcan_usb_fd_ethtool_ops, + /* timestamps usage */ .ts_used_bits = 32, .ts_period = 1000000, /* calibration period in ts. */ @@ -1269,6 +1300,8 @@ const struct peak_usb_adapter pcan_usb_x6 = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), + .ethtool_ops = &pcan_usb_fd_ethtool_ops, + /* timestamps usage */ .ts_used_bits = 32, .ts_period = 1000000, /* calibration period in ts. */ diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c index 275087c39602..ff740b4203fa 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c @@ -9,6 +9,7 @@ #include <linux/netdevice.h> #include <linux/usb.h> #include <linux/module.h> +#include <linux/ethtool.h> #include <linux/can.h> #include <linux/can/dev.h> @@ -908,7 +909,7 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev) usb_if->dev[dev->ctrl_idx] = dev; /* set LED in default state (end of init phase) */ - pcan_usb_pro_set_led(dev, 0, 1); + pcan_usb_pro_set_led(dev, FW_USBPRO_LED_DEVICE, 1); kfree(bi); kfree(fi); @@ -992,6 +993,35 @@ int pcan_usb_pro_probe(struct usb_interface *intf) return 0; } +static int pcan_usb_pro_set_phys_id(struct net_device *netdev, + enum ethtool_phys_id_state state) +{ + struct peak_usb_device *dev = netdev_priv(netdev); + int err = 0; + + switch (state) { + case ETHTOOL_ID_ACTIVE: + /* fast blinking forever */ + err = pcan_usb_pro_set_led(dev, FW_USBPRO_LED_BLINK_FAST, + 0xffffffff); + break; + + case ETHTOOL_ID_INACTIVE: + /* restore LED default */ + err = pcan_usb_pro_set_led(dev, FW_USBPRO_LED_DEVICE, 1); + break; + + default: + break; + } + + return err; +} + +static const struct ethtool_ops pcan_usb_pro_ethtool_ops = { + .set_phys_id = pcan_usb_pro_set_phys_id, +}; + /* * describe the PCAN-USB Pro adapter */ @@ -1020,6 +1050,8 @@ const struct peak_usb_adapter pcan_usb_pro = { /* size of device private data */ .sizeof_dev_private = sizeof(struct pcan_usb_pro_device), + .ethtool_ops = &pcan_usb_pro_ethtool_ops, + /* timestamps usage */ .ts_used_bits = 32, .ts_period = 1000000, /* calibration period in ts. */ diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h index 6bb12357d078..421104ee29f4 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h @@ -115,6 +115,12 @@ struct __packed pcan_usb_pro_devid { __le32 serial_num; }; +#define FW_USBPRO_LED_DEVICE 0x00 +#define FW_USBPRO_LED_BLINK_FAST 0x01 +#define FW_USBPRO_LED_BLINK_SLOW 0x02 +#define FW_USBPRO_LED_ON 0x03 +#define FW_USBPRO_LED_OFF 0x04 + struct __packed pcan_usb_pro_setled { u8 data_type; u8 channel; -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() 2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean @ 2021-03-09 11:20 ` Marc Kleine-Budde 2021-03-09 12:15 ` Marc Kleine-Budde 1 sibling, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 11:20 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 564 bytes --] On 09.03.2021 09:21:26, Stephane Grosjean wrote: > This patch makes it possible to specifically flash the LED of a CAN port > of the CAN-USB interfaces of PEAK-System. > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> Added this one to linux-can-next/testing. 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() 2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean 2021-03-09 11:20 ` Marc Kleine-Budde @ 2021-03-09 12:15 ` Marc Kleine-Budde 2021-03-19 8:35 ` Marc Kleine-Budde 1 sibling, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 12:15 UTC (permalink / raw) To: Stephane Grosjean, linux-can Mailing List [-- Attachment #1.1: Type: text/plain, Size: 10172 bytes --] On 3/9/21 9:21 AM, Stephane Grosjean wrote: > This patch makes it possible to specifically flash the LED of a CAN port > of the CAN-USB interfaces of PEAK-System. > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > --- > drivers/net/can/usb/peak_usb/pcan_usb.c | 47 ++++++++++++++++++++ > drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 ++ > drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 + > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 33 ++++++++++++++ > drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 34 +++++++++++++- > drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 6 +++ > 6 files changed, 125 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c > index e6c1e5d33924..aa89c1119e27 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c > @@ -11,6 +11,7 @@ > #include <linux/netdevice.h> > #include <linux/usb.h> > #include <linux/module.h> > +#include <linux/ethtool.h> > > #include <linux/can.h> > #include <linux/can/dev.h> > @@ -42,6 +43,7 @@ MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB adapter"); > #define PCAN_USB_CMD_REGISTER 9 > #define PCAN_USB_CMD_EXT_VCC 10 > #define PCAN_USB_CMD_ERR_FR 11 > +#define PCAN_USB_CMD_LED 12 > > /* PCAN_USB_CMD_SET_BUS number arg */ > #define PCAN_USB_BUS_XCVER 2 > @@ -250,6 +252,15 @@ static int pcan_usb_set_ext_vcc(struct peak_usb_device *dev, u8 onoff) > return pcan_usb_send_cmd(dev, PCAN_USB_CMD_EXT_VCC, PCAN_USB_SET, args); > } > > +static int pcan_usb_set_led(struct peak_usb_device *dev, u8 onoff) > +{ > + u8 args[PCAN_USB_CMD_ARGS_LEN] = { > + [0] = !!onoff, > + }; > + > + return pcan_usb_send_cmd(dev, PCAN_USB_CMD_LED, PCAN_USB_SET, args); > +} > + > /* > * set bittiming value to can > */ > @@ -973,6 +984,40 @@ static int pcan_usb_probe(struct usb_interface *intf) > return 0; > } > > +static int pcan_usb_set_phys_id(struct net_device *netdev, > + enum ethtool_phys_id_state state) > +{ > + struct peak_usb_device *dev = netdev_priv(netdev); > + int err = 0; > + > + switch (state) { > + case ETHTOOL_ID_ACTIVE: > + /* call ON/OFF twice a second */ > + return 2; > + > + case ETHTOOL_ID_OFF: > + err = pcan_usb_set_led(dev, 0); > + break; > + > + case ETHTOOL_ID_ON: > + fallthrough; > + > + case ETHTOOL_ID_INACTIVE: > + /* restore LED default */ > + err = pcan_usb_set_led(dev, 1); > + break; > + > + default: > + break; > + } > + > + return err; > +} > + > +static const struct ethtool_ops pcan_usb_ethtool_ops = { > + .set_phys_id = pcan_usb_set_phys_id, > +}; > + > /* > * describe the PCAN-USB adapter > */ > @@ -1003,6 +1048,8 @@ const struct peak_usb_adapter pcan_usb = { > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb), > > + .ethtool_ops = &pcan_usb_ethtool_ops, > + > /* timestamps usage */ > .ts_used_bits = 16, > .ts_period = 24575, /* calibration period in ts. */ > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > index 573b11559d73..685c59e2cfd3 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > @@ -14,6 +14,7 @@ > #include <linux/module.h> > #include <linux/netdevice.h> > #include <linux/usb.h> > +#include <linux/ethtool.h> > > #include <linux/can.h> > #include <linux/can/dev.h> > @@ -820,6 +821,9 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter, > > netdev->flags |= IFF_ECHO; /* we support local echo */ > > + /* add ethtool support */ > + netdev->ethtool_ops = peak_usb_adapter->ethtool_ops; > + > init_usb_anchor(&dev->rx_submitted); > > init_usb_anchor(&dev->tx_submitted); > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h > index 4b1528a42a7b..e15b4c78f309 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h > @@ -46,6 +46,8 @@ struct peak_usb_adapter { > const struct can_bittiming_const * const data_bittiming_const; > unsigned int ctrl_count; > > + const struct ethtool_ops *ethtool_ops; > + > int (*intf_probe)(struct usb_interface *intf); > > int (*dev_init)(struct peak_usb_device *dev); > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > index f347ecc79aef..6183a42f6491 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > @@ -7,6 +7,7 @@ > #include <linux/netdevice.h> > #include <linux/usb.h> > #include <linux/module.h> > +#include <linux/ethtool.h> > > #include <linux/can.h> > #include <linux/can/dev.h> > @@ -1009,6 +1010,31 @@ static void pcan_usb_fd_free(struct peak_usb_device *dev) > } > } > > +/* blink LED's */ > +static int pcan_usb_fd_set_phys_id(struct net_device *netdev, > + enum ethtool_phys_id_state state) > +{ > + struct peak_usb_device *dev = netdev_priv(netdev); > + int err = 0; > + > + switch (state) { > + case ETHTOOL_ID_ACTIVE: > + err = pcan_usb_fd_set_can_led(dev, PCAN_UFD_LED_FAST); > + break; > + case ETHTOOL_ID_INACTIVE: > + err = pcan_usb_fd_set_can_led(dev, PCAN_UFD_LED_DEF); > + break; > + default: > + break; > + } > + > + return err; > +} > + > +static const struct ethtool_ops pcan_usb_fd_ethtool_ops = { > + .set_phys_id = pcan_usb_fd_set_phys_id, > +}; > + > /* describes the PCAN-USB FD adapter */ > static const struct can_bittiming_const pcan_usb_fd_const = { > .name = "pcan_usb_fd", > @@ -1050,6 +1076,8 @@ const struct peak_usb_adapter pcan_usb_fd = { > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), > > + .ethtool_ops = &pcan_usb_fd_ethtool_ops, > + > /* timestamps usage */ > .ts_used_bits = 32, > .ts_period = 1000000, /* calibration period in ts. */ > @@ -1122,6 +1150,7 @@ const struct peak_usb_adapter pcan_usb_chip = { > > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), > + .ethtool_ops = &pcan_usb_fd_ethtool_ops, > > /* timestamps usage */ > .ts_used_bits = 32, > @@ -1196,6 +1225,8 @@ const struct peak_usb_adapter pcan_usb_pro_fd = { > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), > > + .ethtool_ops = &pcan_usb_fd_ethtool_ops, > + > /* timestamps usage */ > .ts_used_bits = 32, > .ts_period = 1000000, /* calibration period in ts. */ > @@ -1269,6 +1300,8 @@ const struct peak_usb_adapter pcan_usb_x6 = { > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb_fd_device), > > + .ethtool_ops = &pcan_usb_fd_ethtool_ops, > + > /* timestamps usage */ > .ts_used_bits = 32, > .ts_period = 1000000, /* calibration period in ts. */ > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c > index 275087c39602..ff740b4203fa 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c > @@ -9,6 +9,7 @@ > #include <linux/netdevice.h> > #include <linux/usb.h> > #include <linux/module.h> > +#include <linux/ethtool.h> > > #include <linux/can.h> > #include <linux/can/dev.h> > @@ -908,7 +909,7 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev) > usb_if->dev[dev->ctrl_idx] = dev; > > /* set LED in default state (end of init phase) */ > - pcan_usb_pro_set_led(dev, 0, 1); > + pcan_usb_pro_set_led(dev, FW_USBPRO_LED_DEVICE, 1); > > kfree(bi); > kfree(fi); > @@ -992,6 +993,35 @@ int pcan_usb_pro_probe(struct usb_interface *intf) > return 0; > } > > +static int pcan_usb_pro_set_phys_id(struct net_device *netdev, > + enum ethtool_phys_id_state state) > +{ > + struct peak_usb_device *dev = netdev_priv(netdev); > + int err = 0; > + > + switch (state) { > + case ETHTOOL_ID_ACTIVE: > + /* fast blinking forever */ > + err = pcan_usb_pro_set_led(dev, FW_USBPRO_LED_BLINK_FAST, > + 0xffffffff); > + break; > + > + case ETHTOOL_ID_INACTIVE: > + /* restore LED default */ > + err = pcan_usb_pro_set_led(dev, FW_USBPRO_LED_DEVICE, 1); > + break; > + > + default: > + break; > + } > + > + return err; > +} > + > +static const struct ethtool_ops pcan_usb_pro_ethtool_ops = { > + .set_phys_id = pcan_usb_pro_set_phys_id, > +}; > + > /* > * describe the PCAN-USB Pro adapter > */ > @@ -1020,6 +1050,8 @@ const struct peak_usb_adapter pcan_usb_pro = { > /* size of device private data */ > .sizeof_dev_private = sizeof(struct pcan_usb_pro_device), > > + .ethtool_ops = &pcan_usb_pro_ethtool_ops, > + > /* timestamps usage */ > .ts_used_bits = 32, > .ts_period = 1000000, /* calibration period in ts. */ > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > index 6bb12357d078..421104ee29f4 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > @@ -115,6 +115,12 @@ struct __packed pcan_usb_pro_devid { > __le32 serial_num; > }; > > +#define FW_USBPRO_LED_DEVICE 0x00 > +#define FW_USBPRO_LED_BLINK_FAST 0x01 > +#define FW_USBPRO_LED_BLINK_SLOW 0x02 > +#define FW_USBPRO_LED_ON 0x03 > +#define FW_USBPRO_LED_OFF 0x04 What about replacing the FW_ with PCAN_, so that we're using the same prefix within the driver? I can do this while applying. > + > struct __packed pcan_usb_pro_setled { > u8 data_type; > u8 channel; > 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 #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() 2021-03-09 12:15 ` Marc Kleine-Budde @ 2021-03-19 8:35 ` Marc Kleine-Budde 0 siblings, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-19 8:35 UTC (permalink / raw) To: Stephane Grosjean, linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 1099 bytes --] On 09.03.2021 13:15:27, Marc Kleine-Budde wrote: > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > > index 6bb12357d078..421104ee29f4 100644 > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h > > @@ -115,6 +115,12 @@ struct __packed pcan_usb_pro_devid { > > __le32 serial_num; > > }; > > > > +#define FW_USBPRO_LED_DEVICE 0x00 > > +#define FW_USBPRO_LED_BLINK_FAST 0x01 > > +#define FW_USBPRO_LED_BLINK_SLOW 0x02 > > +#define FW_USBPRO_LED_ON 0x03 > > +#define FW_USBPRO_LED_OFF 0x04 > > What about replacing the FW_ with PCAN_, so that we're using the same prefix > within the driver? I can do this while applying. I've replaced the FW_ with PCAN_ while applying the patch. 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-09 8:21 [PATCH 0/3] can/peak_usb: Added improvement to the peak_usb driver Stephane Grosjean 2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean @ 2021-03-09 8:21 ` Stephane Grosjean 2021-03-09 11:07 ` Marc Kleine-Budde 2021-03-09 15:28 ` Marc Kleine-Budde 2021-03-09 8:21 ` [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode Stephane Grosjean 2 siblings, 2 replies; 24+ messages in thread From: Stephane Grosjean @ 2021-03-09 8:21 UTC (permalink / raw) To: linux-can Mailing List; +Cc: Stephane Grosjean Since the peak_usb driver also supports the CAN-USB interfaces "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds their names to the list of explicitly supported devices. Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> --- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index 6183a42f6491..8e6250c4c417 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -19,6 +19,8 @@ MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); #define PCAN_USBPROFD_CHANNEL_COUNT 2 #define PCAN_USBFD_CHANNEL_COUNT 1 -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-09 8:21 ` [PATCH 2/3] can/peak_usb: add forgotten supported devices Stephane Grosjean @ 2021-03-09 11:07 ` Marc Kleine-Budde 2021-03-09 14:22 ` Stéphane Grosjean 2021-03-09 15:28 ` Marc Kleine-Budde 1 sibling, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 11:07 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 628 bytes --] On 09.03.2021 09:21:27, Stephane Grosjean wrote: > Since the peak_usb driver also supports the CAN-USB interfaces > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > their names to the list of explicitly supported devices. Can you look up which patch added support for these interfaces? Then we can add a Fixes tag. 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices @ 2021-03-09 14:22 ` Stéphane Grosjean 2021-03-09 15:27 ` Marc Kleine-Budde 0 siblings, 1 reply; 24+ messages in thread From: Stéphane Grosjean @ 2021-03-09 14:22 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: linux-can Mailing List Hi Marc, PCAN-USB X6 support has been added by https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f00b534ded60bd0a23c2fa8dec4ece52aa7d235f PCAN-Chip USB support has been added by https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea8b65b596d78969629562f9728f76cbf565fbec Should I update the patch description with these commit Id and push a v2? --- Stéphane From: Marc Kleine-Budde Sent: Tuesday, March 9, 2021 12:07 To: Stéphane Grosjean Cc: linux-can Mailing List Subject: Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices On 09.03.2021 09:21:27, Stephane Grosjean wrote: > Since the peak_usb driver also supports the CAN-USB interfaces > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > their names to the list of explicitly supported devices. Can you look up which patch added support for these interfaces? Then we can add a Fixes tag. 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 | -- PEAK-System Technik GmbH Sitz der Gesellschaft Darmstadt - HRB 9183 Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm Unsere Datenschutzerklaerung mit wichtigen Hinweisen zur Behandlung personenbezogener Daten finden Sie unter www.peak-system.com/Datenschutz.483.0.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-09 14:22 ` Stéphane Grosjean @ 2021-03-09 15:27 ` Marc Kleine-Budde 0 siblings, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 15:27 UTC (permalink / raw) To: Stéphane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 808 bytes --] On 09.03.2021 14:22:38, Stéphane Grosjean wrote: > Hi Marc, > > PCAN-USB X6 support has been added by > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f00b534ded60bd0a23c2fa8dec4ece52aa7d235f > > PCAN-Chip USB support has been added by > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea8b65b596d78969629562f9728f76cbf565fbec > > Should I update the patch description with these commit Id and push a > v2? No need, I've updated the patch. regards, 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-09 8:21 ` [PATCH 2/3] can/peak_usb: add forgotten supported devices Stephane Grosjean 2021-03-09 11:07 ` Marc Kleine-Budde @ 2021-03-09 15:28 ` Marc Kleine-Budde 2021-03-19 8:39 ` Marc Kleine-Budde 1 sibling, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 15:28 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 1300 bytes --] On 09.03.2021 09:21:27, Stephane Grosjean wrote: > Since the peak_usb driver also supports the CAN-USB interfaces > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > their names to the list of explicitly supported devices. > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > --- > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > index 6183a42f6491..8e6250c4c417 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > @@ -19,6 +19,8 @@ > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); I've exchanged these, to correspond the order of the device ids. 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-09 15:28 ` Marc Kleine-Budde @ 2021-03-19 8:39 ` Marc Kleine-Budde 2021-03-19 9:47 ` Vincent MAILHOL 0 siblings, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-19 8:39 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 1589 bytes --] On 09.03.2021 16:28:37, Marc Kleine-Budde wrote: > On 09.03.2021 09:21:27, Stephane Grosjean wrote: > > Since the peak_usb driver also supports the CAN-USB interfaces > > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > > their names to the list of explicitly supported devices. > > > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > --- > > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > index 6183a42f6491..8e6250c4c417 100644 > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > @@ -19,6 +19,8 @@ > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); > > I've exchanged these, to correspond the order of the device ids. Funny side note: MODULE_SUPPORTED_DEVICE was a noop define. All uses have been globally removed from Linus' tree after this patch hit linux-net/master, but before it landed in Linus' tree. 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-19 8:39 ` Marc Kleine-Budde @ 2021-03-19 9:47 ` Vincent MAILHOL 2021-03-19 9:56 ` Marc Kleine-Budde 0 siblings, 1 reply; 24+ messages in thread From: Vincent MAILHOL @ 2021-03-19 9:47 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: Stephane Grosjean, linux-can Mailing List On Fri. 19 Mar 2021 at 17:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 09.03.2021 16:28:37, Marc Kleine-Budde wrote: > > On 09.03.2021 09:21:27, Stephane Grosjean wrote: > > > Since the peak_usb driver also supports the CAN-USB interfaces > > > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > > > their names to the list of explicitly supported devices. > > > > > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > > --- > > > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > index 6183a42f6491..8e6250c4c417 100644 > > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > @@ -19,6 +19,8 @@ > > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); > > > > I've exchanged these, to correspond the order of the device ids. > > Funny side note: > MODULE_SUPPORTED_DEVICE was a noop define. All uses have been globally > removed from Linus' tree after this patch hit linux-net/master, but > before it landed in Linus' tree. Silly question but does it mean that we should not use MODULE_SUPPORTED_DEVICE in newly submitted patches? After seeing Stéphane's patch, I added it to my driver. Even if it is a noop define, it adds meta information in the source code so I was inclined to keep it. Yours sincerely, Vincent ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-19 9:47 ` Vincent MAILHOL @ 2021-03-19 9:56 ` Marc Kleine-Budde 2021-03-19 10:07 ` Vincent MAILHOL 0 siblings, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-19 9:56 UTC (permalink / raw) To: Vincent MAILHOL; +Cc: Stephane Grosjean, linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 2367 bytes --] On 19.03.2021 18:47:06, Vincent MAILHOL wrote: > On Fri. 19 Mar 2021 at 17:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > On 09.03.2021 16:28:37, Marc Kleine-Budde wrote: > > > On 09.03.2021 09:21:27, Stephane Grosjean wrote: > > > > Since the peak_usb driver also supports the CAN-USB interfaces > > > > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > > > > their names to the list of explicitly supported devices. > > > > > > > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > > > --- > > > > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > index 6183a42f6491..8e6250c4c417 100644 > > > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > @@ -19,6 +19,8 @@ > > > > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); > > > > > > I've exchanged these, to correspond the order of the device ids. > > > > Funny side note: > > MODULE_SUPPORTED_DEVICE was a noop define. All uses have been globally > > removed from Linus' tree after this patch hit linux-net/master, but > > before it landed in Linus' tree. > > Silly question but does it mean that we should not use > MODULE_SUPPORTED_DEVICE in newly submitted patches? ACK - It's been removed from Linus' tree, see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6417f03132a6952cd17ddd8eaddbac92b61b17e0 > After seeing Stéphane's patch, I added it to my driver. Even if it is > a noop define, it adds meta information in the source code so I was > inclined to keep it. As the noop define has been removed, the driver will no longer compile. regards, 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-19 9:56 ` Marc Kleine-Budde @ 2021-03-19 10:07 ` Vincent MAILHOL 2021-03-19 12:00 ` Stéphane Grosjean 0 siblings, 1 reply; 24+ messages in thread From: Vincent MAILHOL @ 2021-03-19 10:07 UTC (permalink / raw) To: Marc Kleine-Budde; +Cc: Stephane Grosjean, linux-can Mailing List On Fri. 19 mars 2021 at 18:56, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 19.03.2021 18:47:06, Vincent MAILHOL wrote: > > On Fri. 19 Mar 2021 at 17:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > > On 09.03.2021 16:28:37, Marc Kleine-Budde wrote: > > > > On 09.03.2021 09:21:27, Stephane Grosjean wrote: > > > > > Since the peak_usb driver also supports the CAN-USB interfaces > > > > > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > > > > > their names to the list of explicitly supported devices. > > > > > > > > > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > > > > --- > > > > > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > index 6183a42f6491..8e6250c4c417 100644 > > > > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > @@ -19,6 +19,8 @@ > > > > > > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); > > > > > > > > I've exchanged these, to correspond the order of the device ids. > > > > > > Funny side note: > > > MODULE_SUPPORTED_DEVICE was a noop define. All uses have been globally > > > removed from Linus' tree after this patch hit linux-net/master, but > > > before it landed in Linus' tree. > > > > Silly question but does it mean that we should not use > > MODULE_SUPPORTED_DEVICE in newly submitted patches? > > ACK - It's been removed from Linus' tree, see: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6417f03132a6952cd17ddd8eaddbac92b61b17e0 > > > After seeing Stéphane's patch, I added it to my driver. Even if it is > > a noop define, it adds meta information in the source code so I was > > inclined to keep it. > > As the noop define has been removed, the driver will no longer compile. Got it, thanks for the link to the commit! Yours sincerely, Vincent ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 2/3] can/peak_usb: add forgotten supported devices 2021-03-19 10:07 ` Vincent MAILHOL @ 2021-03-19 12:00 ` Stéphane Grosjean 0 siblings, 0 replies; 24+ messages in thread From: Stéphane Grosjean @ 2021-03-19 12:00 UTC (permalink / raw) To: Vincent MAILHOL, Marc Kleine-Budde; +Cc: linux-can Mailing List Hi, Sorry guys for all that noise... :-/ -- Stéphane De : Vincent MAILHOL <mailhol.vincent@wanadoo.fr> Envoyé : vendredi 19 mars 2021 11:07 À : Marc Kleine-Budde <mkl@pengutronix.de> Cc : Stéphane Grosjean <s.grosjean@peak-system.com>; linux-can Mailing List <linux-can@vger.kernel.org> Objet : Re: [PATCH 2/3] can/peak_usb: add forgotten supported devices On Fri. 19 mars 2021 at 18:56, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 19.03.2021 18:47:06, Vincent MAILHOL wrote: > > On Fri. 19 Mar 2021 at 17:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > > On 09.03.2021 16:28:37, Marc Kleine-Budde wrote: > > > > On 09.03.2021 09:21:27, Stephane Grosjean wrote: > > > > > Since the peak_usb driver also supports the CAN-USB interfaces > > > > > "PCAN-USB X6" and "PCAN-Chip USB" from PEAK-System GmbH, this patch adds > > > > > their names to the list of explicitly supported devices. > > > > > > > > > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > > > > --- > > > > > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > index 6183a42f6491..8e6250c4c417 100644 > > > > > --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c > > > > > @@ -19,6 +19,8 @@ > > > > > > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB FD adapter"); > > > > > MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro FD adapter"); > > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB X6 adapter"); > > > > > +MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-Chip USB"); > > > > > > > > I've exchanged these, to correspond the order of the device ids. > > > > > > Funny side note: > > > MODULE_SUPPORTED_DEVICE was a noop define. All uses have been globally > > > removed from Linus' tree after this patch hit linux-net/master, but > > > before it landed in Linus' tree. > > > > Silly question but does it mean that we should not use > > MODULE_SUPPORTED_DEVICE in newly submitted patches? > > ACK - It's been removed from Linus' tree, see: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6417f03132a6952cd17ddd8eaddbac92b61b17e0 > > > After seeing Stéphane's patch, I added it to my driver. Even if it is > > a noop define, it adds meta information in the source code so I was > > inclined to keep it. > > As the noop define has been removed, the driver will no longer compile. Got it, thanks for the link to the commit! Yours sincerely, Vincent -- PEAK-System Technik GmbH Sitz der Gesellschaft Darmstadt - HRB 9183 Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm Unsere Datenschutzerklaerung mit wichtigen Hinweisen zur Behandlung personenbezogener Daten finden Sie unter www.peak-system.com/Datenschutz.483.0.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 8:21 [PATCH 0/3] can/peak_usb: Added improvement to the peak_usb driver Stephane Grosjean 2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean 2021-03-09 8:21 ` [PATCH 2/3] can/peak_usb: add forgotten supported devices Stephane Grosjean @ 2021-03-09 8:21 ` Stephane Grosjean 2021-03-09 10:36 ` Marc Kleine-Budde 2021-03-19 10:01 ` Marc Kleine-Budde 2 siblings, 2 replies; 24+ messages in thread From: Stephane Grosjean @ 2021-03-09 8:21 UTC (permalink / raw) To: linux-can Mailing List; +Cc: Stephane Grosjean This patch adds "ONE-SHOT" mode support to the following CAN-USB PEAK-System GmbH interfaces: - PCAN-USB X6 - PCAN-USB FD - PCAN-USB Pro FD - PCAN-Chip USB - PCAN-USB Pro Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> --- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 12 ++++++++---- drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 12 +++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c index 8e6250c4c417..0e3bb3a4945d 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c @@ -779,6 +779,10 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev, tx_msg_flags |= PUCAN_MSG_RTR; } + /* Single-Shot frame */ + if (dev->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) + tx_msg_flags |= PUCAN_MSG_SINGLE_SHOT; + tx_msg->flags = cpu_to_le16(tx_msg_flags); tx_msg->channel_dlc = PUCAN_MSG_CHANNEL_DLC(dev->ctrl_idx, dlc); memcpy(tx_msg->d, cfd->data, cfd->len); @@ -1068,7 +1072,7 @@ const struct peak_usb_adapter pcan_usb_fd = { .ctrl_count = PCAN_USBFD_CHANNEL_COUNT, .ctrlmode_supported = CAN_CTRLMODE_FD | CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY | - CAN_CTRLMODE_CC_LEN8_DLC, + CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_CC_LEN8_DLC, .clock = { .freq = PCAN_UFD_CRYSTAL_HZ, }, @@ -1143,7 +1147,7 @@ const struct peak_usb_adapter pcan_usb_chip = { .ctrl_count = PCAN_USBFD_CHANNEL_COUNT, .ctrlmode_supported = CAN_CTRLMODE_FD | CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY | - CAN_CTRLMODE_CC_LEN8_DLC, + CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_CC_LEN8_DLC, .clock = { .freq = PCAN_UFD_CRYSTAL_HZ, }, @@ -1217,7 +1221,7 @@ const struct peak_usb_adapter pcan_usb_pro_fd = { .ctrl_count = PCAN_USBPROFD_CHANNEL_COUNT, .ctrlmode_supported = CAN_CTRLMODE_FD | CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY | - CAN_CTRLMODE_CC_LEN8_DLC, + CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_CC_LEN8_DLC, .clock = { .freq = PCAN_UFD_CRYSTAL_HZ, }, @@ -1292,7 +1296,7 @@ const struct peak_usb_adapter pcan_usb_x6 = { .ctrl_count = PCAN_USBPROFD_CHANNEL_COUNT, .ctrlmode_supported = CAN_CTRLMODE_FD | CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY | - CAN_CTRLMODE_CC_LEN8_DLC, + CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_CC_LEN8_DLC, .clock = { .freq = PCAN_UFD_CRYSTAL_HZ, }, diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c index ff740b4203fa..5b098d1e3746 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c @@ -39,6 +39,7 @@ MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro adapter"); #define PCAN_USBPRO_RTR 0x01 #define PCAN_USBPRO_EXT 0x02 +#define PCAN_USBPRO_SS 0x08 #define PCAN_USBPRO_CMD_BUFFER_SIZE 512 @@ -779,9 +780,13 @@ static int pcan_usb_pro_encode_msg(struct peak_usb_device *dev, flags = 0; if (cf->can_id & CAN_EFF_FLAG) - flags |= 0x02; + flags |= PCAN_USBPRO_EXT; if (cf->can_id & CAN_RTR_FLAG) - flags |= 0x01; + flags |= PCAN_USBPRO_RTR; + + /* Single-Shot frame */ + if (dev->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) + flags |= PCAN_USBPRO_SS; pcan_msg_add_rec(&usb_msg, data_type, 0, flags, len, cf->can_id, cf->data); @@ -1041,7 +1046,8 @@ const struct peak_usb_adapter pcan_usb_pro = { .name = "PCAN-USB Pro", .device_id = PCAN_USBPRO_PRODUCT_ID, .ctrl_count = PCAN_USBPRO_CHANNEL_COUNT, - .ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY, + .ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY | + CAN_CTRLMODE_ONE_SHOT, .clock = { .freq = PCAN_USBPRO_CRYSTAL_HZ, }, -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 8:21 ` [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode Stephane Grosjean @ 2021-03-09 10:36 ` Marc Kleine-Budde 2021-03-09 10:53 ` Oliver Hartkopp 2021-03-19 10:01 ` Marc Kleine-Budde 1 sibling, 1 reply; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 10:36 UTC (permalink / raw) To: Stephane Grosjean, linux-can Mailing List [-- Attachment #1.1: Type: text/plain, Size: 693 bytes --] On 3/9/21 9:21 AM, Stephane Grosjean wrote: > This patch adds "ONE-SHOT" mode support to the following CAN-USB > PEAK-System GmbH interfaces: > - PCAN-USB X6 > - PCAN-USB FD > - PCAN-USB Pro FD > - PCAN-Chip USB > - PCAN-USB Pro > > Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> What happens if in one shot mode and the frame is not send? Who takes care of the echo skb? regards, 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 #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 10:36 ` Marc Kleine-Budde @ 2021-03-09 10:53 ` Oliver Hartkopp 2021-03-09 10:58 ` Marc Kleine-Budde 0 siblings, 1 reply; 24+ messages in thread From: Oliver Hartkopp @ 2021-03-09 10:53 UTC (permalink / raw) To: Marc Kleine-Budde, Stephane Grosjean, linux-can Mailing List On 09.03.21 11:36, Marc Kleine-Budde wrote: > On 3/9/21 9:21 AM, Stephane Grosjean wrote: >> This patch adds "ONE-SHOT" mode support to the following CAN-USB >> PEAK-System GmbH interfaces: >> - PCAN-USB X6 >> - PCAN-USB FD >> - PCAN-USB Pro FD >> - PCAN-Chip USB >> - PCAN-USB Pro >> >> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com> > > What happens if in one shot mode and the frame is not send? Who takes care of > the echo skb? ONE-SHOT only means that the CAN controller would not retry the transmission when e.g. loosing the arbitration or getting an error flag. The sja1000 does it this way, when the TX interrupt flag is set in the interrupt register: if (isrc & IRQ_TI) { /* transmission buffer released */ if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT && !(status & SR_TCS)) { stats->tx_errors++; can_free_echo_skb(dev, 0); } else { /* transmission complete */ stats->tx_bytes += priv->read_reg(priv, SJA1000_FI) & 0xf; stats->tx_packets++; can_get_echo_skb(dev, 0, NULL); } netif_wake_queue(dev); can_led_event(dev, CAN_LED_EVENT_TX); } Do we need to check this for the other drivers? Regards, Oliver ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 10:53 ` Oliver Hartkopp @ 2021-03-09 10:58 ` Marc Kleine-Budde 0 siblings, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 10:58 UTC (permalink / raw) To: Oliver Hartkopp, Stephane Grosjean, linux-can Mailing List [-- Attachment #1.1: Type: text/plain, Size: 1639 bytes --] On 3/9/21 11:53 AM, Oliver Hartkopp wrote: >> What happens if in one shot mode and the frame is not send? Who takes care of >> the echo skb? > > ONE-SHOT only means that the CAN controller would not retry the > transmission when e.g. loosing the arbitration or getting an error flag. > > The sja1000 does it this way, when the TX interrupt flag is set in the > interrupt register: > > if (isrc & IRQ_TI) { > /* transmission buffer released */ > if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT && > !(status & SR_TCS)) { > stats->tx_errors++; > can_free_echo_skb(dev, 0); > } else { > /* transmission complete */ > stats->tx_bytes += > priv->read_reg(priv, > SJA1000_FI) & 0xf; > stats->tx_packets++; > can_get_echo_skb(dev, 0, NULL); > } > netif_wake_queue(dev); > can_led_event(dev, CAN_LED_EVENT_TX); > } > > > Do we need to check this for the other drivers? Yes. 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 #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 8:21 ` [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode Stephane Grosjean 2021-03-09 10:36 ` Marc Kleine-Budde @ 2021-03-19 10:01 ` Marc Kleine-Budde 1 sibling, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-19 10:01 UTC (permalink / raw) To: Stephane Grosjean; +Cc: linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 1065 bytes --] On 09.03.2021 09:21:28, Stephane Grosjean wrote: > --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c > @@ -39,6 +39,7 @@ MODULE_SUPPORTED_DEVICE("PEAK-System PCAN-USB Pro adapter"); > > #define PCAN_USBPRO_RTR 0x01 > #define PCAN_USBPRO_EXT 0x02 > +#define PCAN_USBPRO_SS 0x08 > > #define PCAN_USBPRO_CMD_BUFFER_SIZE 512 > > @@ -779,9 +780,13 @@ static int pcan_usb_pro_encode_msg(struct peak_usb_device *dev, > > flags = 0; > if (cf->can_id & CAN_EFF_FLAG) > - flags |= 0x02; > + flags |= PCAN_USBPRO_EXT; > if (cf->can_id & CAN_RTR_FLAG) > - flags |= 0x01; > + flags |= PCAN_USBPRO_RTR; I've put this change in a separate patch and applied both to linux-can-next/testing. regards, 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode
@ 2021-03-09 14:53 Stéphane Grosjean
2021-03-09 14:58 ` Marc Kleine-Budde
0 siblings, 1 reply; 24+ messages in thread
From: Stéphane Grosjean @ 2021-03-09 14:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Oliver Hartkopp, linux-can Mailing List
In the usb drivers, the echo skb is always released by the USB write complete callback.
--- Stéphane
From: Marc Kleine-Budde
Sent: Tuesday, March 9, 2021 11:58
To: Oliver Hartkopp; Stéphane Grosjean; linux-can Mailing List
Subject: Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode
On 3/9/21 11:53 AM, Oliver Hartkopp wrote:
>> What happens if in one shot mode and the frame is not send? Who takes care of
>> the echo skb?
>
> ONE-SHOT only means that the CAN controller would not retry the
> transmission when e.g. loosing the arbitration or getting an error flag.
>
> The sja1000 does it this way, when the TX interrupt flag is set in the
> interrupt register:
>
> if (isrc & IRQ_TI) {
> /* transmission buffer released */
> if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT &&
> !(status & SR_TCS)) {
> stats->tx_errors++;
> can_free_echo_skb(dev, 0);
> } else {
> /* transmission complete */
> stats->tx_bytes +=
> priv->read_reg(priv,
> SJA1000_FI) & 0xf;
> stats->tx_packets++;
> can_get_echo_skb(dev, 0, NULL);
> }
> netif_wake_queue(dev);
> can_led_event(dev, CAN_LED_EVENT_TX);
> }
>
>
> Do we need to check this for the other drivers?
Yes.
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 |
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-09 14:53 Stéphane Grosjean @ 2021-03-09 14:58 ` Marc Kleine-Budde 0 siblings, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-09 14:58 UTC (permalink / raw) To: Stéphane Grosjean; +Cc: Oliver Hartkopp, linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 662 bytes --] On 09.03.2021 14:53:36, Stéphane Grosjean wrote: > In the usb drivers, the echo skb is always released by the USB write > complete callback. This means there will be an echo_skb_get() when the USB write completes, which is usually before the CAN controller sends the CAN frame. This means a TX complete or an TX abort due to one shot mode will not be reported? regards, 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode
@ 2021-03-10 11:18 Stéphane Grosjean
2021-03-10 11:53 ` Marc Kleine-Budde
0 siblings, 1 reply; 24+ messages in thread
From: Stéphane Grosjean @ 2021-03-10 11:18 UTC (permalink / raw)
To: Marc Kleine-Budde, Oliver Hartkopp; +Cc: linux-can Mailing List
Hello,
To complete the open reflection on this subject: to be perfect, the echo skb should in fact not be released by the USB write completion routine but, like the PCI/PCIe version, on reception in the Rx queue of an echo frame to the one written...
This means :
1 - the core of the peak_usb driver has to be deeply modified
2 - the rx path of the USB interface will be much more loaded, resulting in a higher CPU load
2 - in the case of a one-shot frame, how to manage the fact that this echo frame is never received because the one-shot frame could not be written on the bus? Does this need a garbage collector on the echo skbs?
Is it worth it?
Regards,
--- Stéphane
From: Marc Kleine-Budde
Sent: Tuesday, March 9, 2021 15:58
To: Stéphane Grosjean
Cc: Oliver Hartkopp; linux-can Mailing List
Subject: Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode
On 09.03.2021 14:53:36, Stéphane Grosjean wrote:
> In the usb drivers, the echo skb is always released by the USB write
> complete callback.
This means there will be an echo_skb_get() when the USB write completes,
which is usually before the CAN controller sends the CAN frame.
This means a TX complete or an TX abort due to one shot mode will not be
reported?
regards,
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 |
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode 2021-03-10 11:18 Stéphane Grosjean @ 2021-03-10 11:53 ` Marc Kleine-Budde 0 siblings, 0 replies; 24+ messages in thread From: Marc Kleine-Budde @ 2021-03-10 11:53 UTC (permalink / raw) To: Stéphane Grosjean; +Cc: Oliver Hartkopp, linux-can Mailing List [-- Attachment #1: Type: text/plain, Size: 1472 bytes --] On 10.03.2021 11:18:15, Stéphane Grosjean wrote: > To complete the open reflection on this subject: to be perfect, the > echo skb should in fact not be released by the USB write completion > routine but, like the PCI/PCIe version, on reception in the Rx queue > of an echo frame to the one written... If you don't have a TX complete notification, this would be a workaround to let the TX echo better reflect what's going on on the bus. > This means : > 1 - the core of the peak_usb driver has to be deeply modified > 2 - the rx path of the USB interface will be much more loaded, > resulting in a higher CPU load > 2 - in the case of a one-shot frame, how to manage the fact that this > echo frame is never received because the one-shot frame could not > be written on the bus? Does this need a garbage collector on the > echo skbs? Yes, you have to free the echo skb in case of a no-send due to one-shot mode. Does the HW offer a TX aborted notification? > Is it worth it? If you don't have TX complete or TX abort notifications, then it's not worth it. Better spend time implementing these notifications on the controller side :) regards, 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-03-19 12:01 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-09 8:21 [PATCH 0/3] can/peak_usb: Added improvement to the peak_usb driver Stephane Grosjean 2021-03-09 8:21 ` [PATCH 1/3] can/peak_usb: add support of ethtool set_phys_id() Stephane Grosjean 2021-03-09 11:20 ` Marc Kleine-Budde 2021-03-09 12:15 ` Marc Kleine-Budde 2021-03-19 8:35 ` Marc Kleine-Budde 2021-03-09 8:21 ` [PATCH 2/3] can/peak_usb: add forgotten supported devices Stephane Grosjean 2021-03-09 11:07 ` Marc Kleine-Budde 2021-03-09 14:22 ` Stéphane Grosjean 2021-03-09 15:27 ` Marc Kleine-Budde 2021-03-09 15:28 ` Marc Kleine-Budde 2021-03-19 8:39 ` Marc Kleine-Budde 2021-03-19 9:47 ` Vincent MAILHOL 2021-03-19 9:56 ` Marc Kleine-Budde 2021-03-19 10:07 ` Vincent MAILHOL 2021-03-19 12:00 ` Stéphane Grosjean 2021-03-09 8:21 ` [PATCH 3/3] can/peak_usb: add support of ONE_SHOT mode Stephane Grosjean 2021-03-09 10:36 ` Marc Kleine-Budde 2021-03-09 10:53 ` Oliver Hartkopp 2021-03-09 10:58 ` Marc Kleine-Budde 2021-03-19 10:01 ` Marc Kleine-Budde 2021-03-09 14:53 Stéphane Grosjean 2021-03-09 14:58 ` Marc Kleine-Budde 2021-03-10 11:18 Stéphane Grosjean 2021-03-10 11:53 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).