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

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 05.11.2014 05:21, Thomas Körper wrote:

> already was afraid the module param should be removed :)
> May I print all these infos unconditionally?  ...If someone has
> trouble with the card we really like to see that stuff first.

Oh. It's no problem to print hardware specific information at startup.
But this is IMO only too much because of it's formatting

+	if (dump_infos) {
+		dev_info(dev, "  Probe register: 0x%.8x\n",
+			 acc_ov_read32(ov, ACC_OV_OF_PROBE));
+		dev_info(dev, "  Features: 0x%.4x\n", ov->features);
+		dev_info(dev, "  FPGA Version: 0x%.4x\n",
+			 ov->version);
+		dev_info(dev, "  Strappings: 0x%.4x\n",
+			 acc_ov_read32(ov, ACC_OV_OF_INFO) >> 16);
+		dev_info(dev, "  Active cores: %u\n",
+			 ov->active_cores);
+		dev_info(dev, "  Total cores: %u\n",
+			 ov->total_cores);
+		dev_info(dev, "  Core frequency: %u\n",
+			 ov->core_frequency);
+		dev_info(dev, "  Timestamp frequency: %u\n",
+			 ov->timestamp_frequency);
+	}

What about putting these information in just one or two lines, e.g.

esdacc: FPGA rev x.xx @ 3000Hz (TS @ 400Hz), probe reg x.xx, feat. x.xx, X of Y cores active 

> 
> And about the commit message: maybe
> "This patch adds support for the CAN PCIe/402 card from esd electronic
> system design gmbh. FPGA based CAN controller, supporting up to 4
> nets, MSI and bus mastering, see http://esd.eu/en/products/can-pcie402
> for details."?, or something specific you're missing?

Looks better :-)

Regards,
Oliver