Hey Wolfram, thanks for the review.
On Wed, Feb 4, 2009 at 12:55 PM, Wolfram Sang <w.s...@pengutronix.de> wrote:
>
> I have been told to move such a changelog below the dashed lines,
> because it is not so important for the git-history.
This is only in my email for review purposes. It won't be in the git commit.
>> + status = in_be32(&gpt->regs->status) | MPC52xx_GPT_STATUS_IRQMASK;
>
> Like I mentioned in my first review, the following if-clause is always
> true. You probably mean "& MPC52xx_GPT_STATUS_IRQMASK"?
You're right. I missed this from your first review. Fixed.
>> +/* ---------------------------------------------------------------------
>> + * GPIOLIB hooks
>> + */
>> +#if defined(CONFIG_GPIOLIB)
>> +static inline struct mpc52xx_gpt_priv *gc_to_mpc52xx_gpt(struct gpio_chip
>> *gc)
>> +{
>> + return container_of(to_of_gpio_chip(gc), struct
>> mpc52xx_gpt_priv,of_gc);
>
> Space after ','. As it is now, this is a checkpatch-error. Lines over 80
> chars are a warning.
This is on purpose as I described earlier. Checkpatch is not God, and
the cure is worse than the disease. (Granted, the disease is
equivalent to a running nose, and the cure to cheap kleenex, but hey,
it's still my decision).
>> +#else /* defined(CONFIG_SYSFS) */
>> +static void mpc52xx_gpt_create_attribs(struct mpc52xx_gpt_priv *) { return
>> 0; }
>> +#endif /* defined(CONFIG_SYSFS) */
>
> Hmmm, I still have problems with this sysfs-entry. If you say that this
> is good for debugging, then please make it #DEBUG at least. Imagine
> every driver doing such things, this adds bloat. I'd still rather
> suggest using
>
> ~ memedit /dev/mem
> -> map <MBAR> 0x4000
> -> md 0x600
>
> which does the same and even allows for more (writing for example).
I understand your argument, but I'm still going to leave it in and
leave it enabled for now. I will consider removing it after a release
or too.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
