lkml.org 
[lkml]   [2014]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] mfd: MAX6650/6651 support
    > > Styling i.e nice, neat, easily readable/maintainable code should be
    > > your bread and butter. If styling tires you, perhaps a new career
    > > might be in order. ;)
    >
    > or a new tool to be more professional ...

    Patches accepted.

    > >> >> +#include <linux/i2c.h>
    > >> >> +#include <linux/export.h>
    > >> >
    > >> > Why is this in here?
    > >>
    > >> Because this series was meant for a design review and overall
    > >> direction as opposed to a completely fine tuned patch set. Naturally,
    > >> I agree with the feedback of removing unnecessary header inclusion.
    > >
    > > Don't do that.
    >
    > I will guess that you mean "unnecessary header inclusion is OK"...

    No, I mean don't send sub-standard patches which crap you don't need
    or a copy and pasted mess contained within them.

    > >> >> +struct max6651_dev {
    > >> >> + struct device *dev;
    > >> >> + struct mutex iolock;
    > >> >> +
    > >> >> + struct i2c_client *i2c;
    > >> >
    > >> > Is this used?
    > >>
    > >> Yes, heavily, for reading and writing the registers in the subdevice drivers.
    > >
    > > Can you show me where?
    >
    > Check the gpio driver or even the hwmon in this series. Look at the
    > places where it is using the read/write_reg functions, or you can just
    > check their signature in this patch. I am in the process of
    > refactoring it into regmap as we speak, but it is not painless because
    > I need to get it working for 3.2, too ...

    Can you show me the line where you initialise it?

    > >> >> + int type;
    > >> >
    > >> > Or this?
    > >>
    > >> Absolutely, this identifies the type, which is necessary for
    > >> initializing some corresponding data.
    > >
    > > Can you show me where?
    >
    > Well, you have different number of GPIO pins for starter ...

    Can you show me the line where this variable is used?

    > >> >> +};
    > >> >> +
    > >> >> +enum max6651_types {
    > >> >> + TYPE_MAX6650,
    > >> >> + TYPE_MAX6651,
    > >> >> +};
    > >> >
    > >> > What are you using these for?
    > >>
    > >> See above.
    > >
    > > Can you show me where you are using them?
    >
    > Perhaps, I was not while submitting this change, but the upcoming
    > changes should.

    Again, this adds confusion. Send your best, cleanest, most up-to-date
    code, or all you're doing is wasting people's time.
    --
    Lee Jones
    Linaro STMicroelectronics Landing Team Lead
    Linaro.org │ Open source software for ARM SoCs
    Follow Linaro: Facebook | Twitter | Blog
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2014-01-09 13:01    [W:3.342 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site