| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <522D854F.2030207@pengutronix.de>
Date: Mon, 09 Sep 2013 10:22:39 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Benedikt Spranger <b.spranger@...utronix.de>
CC: netdev@...r.kernel.org,
Alexander Frank <Alexander.Frank@...rspaecher.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Holger Dengler <dengler@...utronix.de>
Subject: Re: [PATCH 01/16] c_can_platform: add FlexCard D-CAN support
On 09/09/2013 09:24 AM, Benedikt Spranger wrote:
> FlexCard supports up to 8 D-CAN devices with a shared DMA-capable receive
> function. Add support for FlexCard D-CAN.
>
> Signed-off-by: Benedikt Spranger <b.spranger@...utronix.de>
What are the prerequisites for this series? This doesn't compile against
current net-next/master (e7d33bb lockref: add ability to mark lockrefs
"dead").
More Comments inline.
Marc
> ---
> drivers/net/can/c_can/c_can.h | 1 +
> drivers/net/can/c_can/c_can_platform.c | 149 ++++++++++++++++++++++++++++++++-
> 2 files changed, 148 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index d2e1c21..d2e2d20 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -148,6 +148,7 @@ enum c_can_dev_id {
> BOSCH_C_CAN_PLATFORM,
> BOSCH_C_CAN,
> BOSCH_D_CAN,
> + BOSCH_D_CAN_FLEXCARD,
> };
>
> /* c_can private data structure */
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index c6f838d..43a3e3f 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -32,6 +32,7 @@
> #include <linux/clk.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/flexcard.h>
>
> #include <linux/can/dev.h>
>
> @@ -81,6 +82,112 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
> writel(val, priv->raminit_ctrlreg);
> }
>
> +static int c_can_rx_pkt(void *p, void *data, size_t len)
> +{
Why are the first two arguments a void *, why do we have len, that's
never used? Has the length already been checked, is it guaranteed >=
sizeof(struct fc_packet_buf)?
> + struct net_device *dev = p;
> + struct net_device_stats *stats = &dev->stats;
> + struct fc_packet_buf *pb = data;
> + union fc_packet_types *pt = &pb->packet;
> + struct can_frame *frame;
> + struct sk_buff *skb;
> + u32 flags, id, state, type;
> +
> + switch (le32_to_cpu(pb->header.type)) {
> + case fc_packet_type_can:
> + skb = alloc_can_skb(dev, &frame);
> + if (!skb) {
> + stats->rx_dropped++;
> + return -ENOMEM;
> + }
> +
> + id = le32_to_cpu(pt->can_packet.id);
> + flags = le32_to_cpu(pt->can_packet.flags);
> +
> + if (id & BIT(29))
> + frame->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> + else
> + frame->can_id = id & CAN_SFF_MASK;
> +
> + if (flags & BIT(13))
> + frame->can_id |= CAN_RTR_FLAG;
> + frame->can_dlc = (flags >> 8) & 0xf;
please use get_can_dlc((flags >> 8) & 0xf)
> + memcpy(frame->data, pt->can_packet.data, frame->can_dlc);
Please don't copy data if you receive an RTR frame. What about the
endianess of the data?
> + netif_receive_skb(skb);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += frame->can_dlc;
> + break;
> +
> + case fc_packet_type_can_error:
> + stats->rx_errors++;
> +
> + skb = alloc_can_err_skb(dev, &frame);
> + if (!skb)
> + return -ENOMEM;
> +
> + type = le32_to_cpu(pt->can_error_packet.type);
> + state = le32_to_cpu(pt->can_error_packet.state);
> +
> + switch (state) {
> + case fc_can_state_warning:
> + frame->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> + frame->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> + frame->can_id |= CAN_ERR_CRTL;
> + break;
> + case fc_can_state_error_passive:
> + frame->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> + frame->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> + frame->can_id |= CAN_ERR_CRTL;
> + break;
> + case fc_can_state_bus_off:
> + frame->can_id |= CAN_ERR_BUSOFF;
> + break;
> + }
> +
> + switch (type) {
> + case fc_can_error_stuff:
> + frame->data[2] |= CAN_ERR_PROT_STUFF;
> + frame->can_id |= CAN_ERR_PROT;
> + break;
> + case fc_can_error_form:
> + frame->data[2] |= CAN_ERR_PROT_FORM;
> + frame->can_id |= CAN_ERR_PROT;
> + break;
> + case fc_can_error_acknowledge:
> + frame->can_id |= CAN_ERR_ACK;
> + break;
> + case fc_can_error_bit1:
> + frame->data[2] |= CAN_ERR_PROT_BIT1;
> + frame->can_id |= CAN_ERR_PROT;
> + break;
> + case fc_can_error_bit0:
> + frame->data[2] |= CAN_ERR_PROT_BIT0;
> + frame->can_id |= CAN_ERR_PROT;
> + break;
> + case fc_can_error_crc:
> + frame->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> + frame->can_id |= CAN_ERR_PROT;
> + break;
> + case fc_can_error_parity:
> + frame->data[1] |= CAN_ERR_PROT_UNSPEC;
> + frame->can_id |= CAN_ERR_CRTL;
> + break;
> + }
> + frame->data[5] = pt->can_error_packet.rx_error_counter;
> + frame->data[6] = pt->can_error_packet.tx_error_counter;
> +
> + stats->rx_bytes += frame->can_dlc;
> + stats->rx_packets++;
> +
> + netif_receive_skb(skb);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static struct platform_device_id c_can_id_table[] = {
> [BOSCH_C_CAN_PLATFORM] = {
> .name = KBUILD_MODNAME,
> @@ -93,6 +200,10 @@ static struct platform_device_id c_can_id_table[] = {
> [BOSCH_D_CAN] = {
> .name = "d_can",
> .driver_data = BOSCH_D_CAN,
> + },
> + [BOSCH_D_CAN_FLEXCARD] = {
> + .name = "flexcard-d_can",
> + .driver_data = BOSCH_D_CAN_FLEXCARD,
> }, {
> }
> };
> @@ -108,7 +219,7 @@ MODULE_DEVICE_TABLE(of, c_can_of_table);
> static int c_can_plat_probe(struct platform_device *pdev)
> {
> int ret;
> - void __iomem *addr;
> + void __iomem *addr, *reset_reg;
> struct net_device *dev;
> struct c_can_priv *priv;
> const struct of_device_id *match;
> @@ -167,6 +278,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
> }
>
> priv = netdev_priv(dev);
> + priv->can.clock.freq = clk_get_rate(clk);
> +
> switch (id->driver_data) {
> case BOSCH_C_CAN:
> priv->regs = reg_map_c_can;
> @@ -199,7 +312,26 @@ static int c_can_plat_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "control memory is not used for raminit\n");
> else
> priv->raminit = c_can_hw_raminit;
> +
Please remove
> break;
> + case BOSCH_D_CAN_FLEXCARD:
> + priv->regs = reg_map_d_can;
> + priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> + priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> + priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> + priv->instance = pdev->id;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + reset_reg = ioremap(res->start, resource_size(res));
> + if (!reset_reg || priv->instance < 0) {
priv->instance in unsinged
> + dev_info(&pdev->dev, "Can't reset device.\n");
> + } else {
> + writel(1 << (4 + priv->instance), addr);
> + udelay(100);
> + }
> + iounmap(reset_reg);
> + priv->can.clock.freq = 16000000;
> +
> default:
> ret = -EINVAL;
> goto exit_free_device;
> @@ -208,7 +340,6 @@ static int c_can_plat_probe(struct platform_device *pdev)
> dev->irq = irq;
> priv->base = addr;
> priv->device = &pdev->dev;
> - priv->can.clock.freq = clk_get_rate(clk);
> priv->priv = clk;
> priv->type = id->driver_data;
>
> @@ -222,10 +353,22 @@ static int c_can_plat_probe(struct platform_device *pdev)
> goto exit_free_device;
> }
>
> + if (id->driver_data == BOSCH_D_CAN_FLEXCARD) {
> + ret = fc_register_rx_pkt(FC_CANIF_OFFSET + pdev->id,
> + dev, c_can_rx_pkt);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "registering RX-CB failed %d\n", ret);
> + goto exit_unregister_device;
> + }
> + }
> +
> dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> KBUILD_MODNAME, priv->base, dev->irq);
> return 0;
>
> +exit_unregister_device:
> + unregister_c_can_dev(dev);
> exit_free_device:
> free_c_can_dev(dev);
> exit_iounmap:
> @@ -246,6 +389,8 @@ static int c_can_plat_remove(struct platform_device *pdev)
> struct c_can_priv *priv = netdev_priv(dev);
> struct resource *mem;
>
> + if (priv->type == BOSCH_D_CAN_FLEXCARD)
> + fc_unregister_rx_pkt(FC_CANIF_OFFSET + priv->instance);
> unregister_c_can_dev(dev);
>
> free_c_can_dev(dev);
>
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 |
Download attachment "signature.asc" of type "application/pgp-signature" (260 bytes)
Powered by blists - more mailing lists