> > +module_param(limit_maxrdreqsz, int, 0644);
>
>type can be bool instead of int here?
Yes, it should be bool.
> > + if ((limit_maxrdreqsz) ||
> > + ((nesdev->nesadapter->phy_type[0] ==
>NES_PHY_TYPE_GLADIUS) &&
> > + (hw_rev == NE020_REV1))) {
> > + nes_debug(NES_DBG_INIT,
>
>This indentation is hard to read, because the then clause visually runs
>into the condition being tested. I generally align the follow-on lines
>to be just inside the opening ( of "if (". And there's no
>reason to put
>parentheses around limit_maxrdreqsz...
I normally don't indent that way either but CodingStyle doc said I
can't use spaces to indent...
"Outside of comments, documentation and except in Kconfig, spaces are never
used for indentation, and the above example is deliberately broken."
> > + pci_read_config_word(pcidev, 0x68, &maxrdreqword);
> > + /* set bits 12-14 to 001b = 256 bytes */
> > + maxrdreqword &= 0x8fff;
> > + maxrdreqword |= 0x1000;
> > + pci_write_config_word(pcidev, 0x68, maxrdreqword);
>
>I would write this as below, using the standard pcie
>interfaces and also
>being defensive so as not to set the max read req to 256 if the
>BIOS/kernel had limited it to 128 already:
>
> if (pcie_get_readrq(pcidev) > 256)
> if (pcie_set_readrq(pcidev, 256)) {
> /* report error */
> }
Thanks for the change. Want a v3 for this patch?
Chien_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
