Messages in this thread | | | Date | Fri, 14 Feb 2014 10:21:52 +0100 | From | Michal Simek <> | Subject | Re: [PATCH 08/14] net: axienet: Removed checkpatch errors/warnings |
| |
On 02/13/2014 04:51 PM, Joe Perches wrote: > On Thu, 2014-02-13 at 08:19 +0100, Michal Simek wrote: >> On 02/13/2014 01:31 AM, Joe Perches wrote: >>> On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote: > > Hi again Michal. > >>>> + netdev_warn(lp->ndev, >>>> + "Could not find clock ethernet controller property."); >>> >>> here too. (and alignment) >> >> This is problematic. I would like to keep 80 char limits and keeping >> this align just break it. That's why I was using tab alignment. >> Probably the solution is just to shorten message. > > (overly long, tiresomely trivial stuff below) > > Your choice. I'm not an 80 column zealot but > please don't shorten the message just to fit > 80 columns if it impacts intelligibility.
I am trying to keep 80 chars and follow subsystem standards.
> Generally, I'd write this something like: > > netdev_warn(lp->ndev, > "Could not find clock ethernet controller property\n"); > > (without the period) which is 83 columns.
ok.
> checkpatch makes exceptions for 80 column line > length maximums for format strings.
yes but testing systems reports it because that 80 chars is still default value.
> > I've no real issue if you indent it back one. > > fyi: this is 77 columns > > netdev_warn(lp->ndev, > "No clock ethernet controller property found\n"); > > About the message itself. > > You dropped the "axienet_mdio_setup" function name. > > I believe the dmesg output will look something like: > > xilinx_temac 0000:01:00.0 (unregistered net_device): Could not find clock ethernet controller property. > xilinx_temac 0000:01:00.0 (unregistered net_device): Setting MDIO clock divisor to default 29 > > Because these 2 messages are effectively linked, > my preference would be to emit them on a single line, > > Something like: > > xilinx_temac 0000:01:00.0 (unregistered net_device): of_get_property("clock-frequency") not found - setting MDIO clock divisor to default 29 > > or > > netdev_warn(lp->ndev, > "of_get_property(\"clock-frequency\") not found - setting MDIO clock divisor to default %u\n", > DEFAULT_CLOCK_DIVISOR); >
But then you are breaking 80 char limits a lot.
Thanks, Michal
| |