Re: [PATCH v2 1/1] can: Add support for esd CAN PCIe/402 card

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi Thomas,

I saw one of your 402 cards on the plugfest last Tuesday :-)

On 24.10.2014 09:41, Thomas Körper wrote:

>   drivers/net/can/esd/esd402_pci.c | 1015 ++++++++++++++++++++++++++++++++++++++
>   drivers/net/can/esd/esdacc.c     |   69 +++
>   drivers/net/can/esd/esdacc.h     |  396 +++++++++++++++

Hm - looking at esd402_pci.c there are many functions which look very generic 
and should therefore be moved to esdacc.c .

E.g.


> +static void pci402_coremsg_rxtxdone(struct acc_core *core,
> +				    const struct acc_bmmsg_rxtxdone *msg)

> +static void pci402_coremsg_txabort(struct acc_core *core,
> +				   const struct acc_bmmsg_txabort *msg)


> +static void pci402_coremsg_overrun(struct acc_core *core,
> +				   const struct acc_bmmsg_overrun *msg)


> +static void pci402_coremsg_buserr(struct acc_core *core,
> +				  const struct acc_bmmsg_buserr *msg)

... etc

esd402_pci.c should mainly contain the PCI and card specific initialization stuff.

E.g. linux/drivers/net/can/sja1000/plx_pci.c is a good example what content 
should be hosted in esd402_pci.c

This is something for esdacc.c too:

> +
> +/* For cards with FPGA Version < PCI402_VERSION_NEWBTR */
> +static const struct can_bittiming_const pci402_bittiming_const_old = {
> +	.name = "pci402_old",
> +	.tseg1_min = 1,
> +	.tseg1_max = 16,
> +	.tseg2_min = 1,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 256,
> +	.brp_inc = 1,
> +};
> +
> +static const struct can_bittiming_const pci402_bittiming_const = {
> +	.name = "pci402",
> +	.tseg1_min = 1,
> +	.tseg1_max = 256,
> +	.tseg2_min = 1,
> +	.tseg2_max = 128,
> +	.sjw_max = 128,
> +	.brp_min = 1,
> +	.brp_max = 256,
> +	.brp_inc = 1,
> +};
> +

Regards,
Oliver