- Use just *one* value per sysfs file
SG - I felt adding entry for each mbx_id will clutter the sysfs.
Is it ok to do that.
+static u32 pruss_intc_init[19][3] = { + {PRUSS_INTC_POLARITY0, PRU_INTC_REGMAP_MASK, 0xFFFFFFFF}, + {PRUSS_INTC_POLARITY1, PRU_INTC_REGMAP_MASK, 0xFFFFFFFF}, + {PRUSS_INTC_TYPE0, PRU_INTC_REGMAP_MASK, 0x1C000000}, + {PRUSS_INTC_TYPE1, PRU_INTC_REGMAP_MASK, 0}, + {PRUSS_INTC_GLBLEN, 0, 1}, + {PRUSS_INTC_HOSTMAP0, PRU_INTC_REGMAP_MASK, 0x03020100}, + {PRUSS_INTC_HOSTMAP1, PRU_INTC_REGMAP_MASK, 0x07060504}, + {PRUSS_INTC_HOSTMAP2, PRU_INTC_REGMAP_MASK, 0x0000908}, + {PRUSS_INTC_CHANMAP0, PRU_INTC_REGMAP_MASK, 0}, + {PRUSS_INTC_CHANMAP8, PRU_INTC_REGMAP_MASK, 0x00020200}, + {PRUSS_INTC_STATIDXCLR, 0, 32}, + {PRUSS_INTC_STATIDXCLR, 0, 19}, + {PRUSS_INTC_ENIDXSET, 0, 19}, + {PRUSS_INTC_STATIDXCLR, 0, 18}, + {PRUSS_INTC_ENIDXSET, 0, 18}, + {PRUSS_INTC_STATIDXCLR, 0, 34}, + {PRUSS_INTC_ENIDXSET, 0, 34}, + {PRUSS_INTC_ENIDXSET, 0, 32}, + {PRUSS_INTC_HOSTINTEN, 0, 5}please add ","Also a struct to describe each entry would improve readability. Then you could also use ARRAY_SIZE.
SG _ I could not follow this, are you recommending that I create a structure with three variables and then create
an array for it.
something like:
const static struct [] = {
{
unsigned int reg_base;
unsigned int reg_mask;
unsigned int reg_val;
},
...
};
+ value = (PRUSS_CAN_GPIO_SETUP_DELAY * + (priv->clk_freq_pru / 1000000) / 1000) / + PRUSS_CAN_DELAY_LOOP_LENGTH;This calculation looks delicate. 64-bit math would be safer.
SG - This one works fine. I am dividing it twice to avoid the problem.
+ pru_can_mask_ints(priv->dev, PRUSS_CAN_TX_PRU_1, false); + pru_can_mask_ints(priv->dev, PRUSS_CAN_RX_PRU_0, false); + can_bus_off(ndev); + dev_dbg(priv->ndev->dev.parent, "Bus off mode\n"); + } + + netif_rx(skb);You should use netif_receive_skb(skb) here as well.
SG - Ok, Will do.
if (PRUSS_CAN_ISR_BIT_ESI &
priv->can_rx_cntx.intr_stat) {
Is more readable.
SG - Ok, Will do.
+ pru_can_gbl_stat_get(priv->dev, + &priv->can_rx_cntx); + pru_can_err(ndev, + priv->can_rx_cntx.intr_stat, + priv->can_rx_cntx.gbl_stat);Please fix bogous indention.
SG - Ok, Will do.
+ + pdata = dev->platform_data; + if (!pdata) { + dev_err(&pdev->dev, "platform data not found\n"); + return -EINVAL; + } + (pdata->setup)();no need fot the ( )
SG - Ok, Will do.
+ } + + priv->ndev = ndev; + priv->dev = dev; + + priv->can.bittiming_const = &pru_can_bittiming_const; + priv->can.do_set_bittiming = pru_can_set_bittiming; + priv->can.do_set_mode = pru_can_set_mode; + priv->can.do_get_state = pru_can_get_state;Please remove that callback. It's not needed as state changes are handled properly.
SG -- Ok, Will do _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
