| Message ID | 1315426421-16243-4-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2011-09-07 at 13:13 -0700, Brian Norris wrote: > These macros can be used instead of including -EUCLEAN and -EBADMSG all > over the place. They should help make code a little bit more readable. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > include/linux/mtd/mtd.h | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index ead70ed..1dd74f4 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -348,4 +348,9 @@ void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size); > > void mtd_erase_callback(struct erase_info *instr); > > +#define is_corrected_ecc_error(error) (error == -EUCLEAN) This is not really an error... > +#define is_uncorrected_ecc_error(error) (error == -EBADMSG) > +#define is_ecc_error(error) ((is_corrected_ecc_error(error) || \ > + is_uncorrected_ecc_error(error))) Could we please have static inline functions instead of macros? I think it is nicer to have mtd_ prefixes, how about these names instead: mtd_is_bitflip() mtd_is_eccerr() mtd_is_bitflip_or_eccerr()
On Sun, 2011-09-11 at 16:57 +0300, Artem Bityutskiy wrote: > > +#define is_corrected_ecc_error(error) (error == -EUCLEAN) > > This is not really an error... > > > +#define is_uncorrected_ecc_error(error) (error == -EBADMSG) > > +#define is_ecc_error(error) ((is_corrected_ecc_error(error) || \ > > + is_uncorrected_ecc_error(error))) > > Could we please have static inline functions instead of macros? > > I think it is nicer to have mtd_ prefixes, how about these names > instead: > > mtd_is_bitflip() > mtd_is_eccerr() > mtd_is_bitflip_or_eccerr() Brian, I was actually waiting for v2 of the patches: 3/14, and 9/14-14/14.
On Sun, Sep 18, 2011 at 9:14 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > On Sun, 2011-09-11 at 16:57 +0300, Artem Bityutskiy wrote: >> > +#define is_corrected_ecc_error(error) (error == -EUCLEAN) >> >> This is not really an error... I suppose I was just considering the names as describing the error code, even if it's not fully an error. >> > +#define is_uncorrected_ecc_error(error) (error == -EBADMSG) >> > +#define is_ecc_error(error) ((is_corrected_ecc_error(error) || \ >> > + is_uncorrected_ecc_error(error))) >> >> Could we please have static inline functions instead of macros? Of course we can! Is there a particular reason? Better type checking? Or just kernel style? I don't think I see a code-size benefit... >> I think it is nicer to have mtd_ prefixes, how about these names >> instead: >> >> mtd_is_bitflip() >> mtd_is_eccerr() >> mtd_is_bitflip_or_eccerr() > > Brian, I was actually waiting for v2 of the patches: 3/14, and > 9/14-14/14. Right, I hadn't settled on a name yet and so I haven't rewritten the patches. I like the fact that your names are shorter. I'm not sure about `mtd_is_bitflip_or_eccerr()', though; the compound name seems a little out of place. But maybe it's just better to be clear on exactly what is covered by the function. Is there a broad category name for bitflips/ECC errors? I just consider them corrected/uncorrected errors in flash, thus the naming I gave. I'll either repost with a better naming scheme or update my patches with the suggested names. Brian
On Mon, 2011-09-19 at 11:43 -0700, Brian Norris wrote: > Right, I hadn't settled on a name yet and so I haven't rewritten the patches. > > I like the fact that your names are shorter. I'm not sure about > `mtd_is_bitflip_or_eccerr()', though; the compound name seems a little > out of place. But maybe it's just better to be clear on exactly what > is covered by the function. Is there a broad category name for > bitflips/ECC errors? I just consider them corrected/uncorrected errors > in flash, thus the naming I gave. > > I'll either repost with a better naming scheme or update my patches > with the suggested names. OK, let's do this sooner than later please, to make sure I have all your work in my tree, which is safer (consider the situation when dwmw2 picks what I have now and upstreams before we put everything in).
On Tue, Sep 20, 2011 at 12:19 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: >> I'll either repost with a better naming scheme or update my patches >> with the suggested names. > > OK, let's do this sooner than later please, to make sure I have all your > work in my tree, which is safer (consider the situation when dwmw2 picks > what I have now and upstreams before we put everything in). OK, I sent the necessary v2 patches. I just went with your names for lack of a better idea. I didn't send v2 of patches 13/14 and 14/14, since there was no dependency. Let me know if I need to resend anything else. Brian
On Tue, 2011-09-20 at 18:40 -0700, Brian Norris wrote: > On Tue, Sep 20, 2011 at 12:19 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > >> I'll either repost with a better naming scheme or update my patches > >> with the suggested names. > > > > OK, let's do this sooner than later please, to make sure I have all your > > work in my tree, which is safer (consider the situation when dwmw2 picks > > what I have now and upstreams before we put everything in). > > OK, I sent the necessary v2 patches. I just went with your names for > lack of a better idea. > > I didn't send v2 of patches 13/14 and 14/14, since there was no > dependency. Let me know if I need to resend anything else. Pushed, thanks for great job!
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index ead70ed..1dd74f4 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -348,4 +348,9 @@ void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size); void mtd_erase_callback(struct erase_info *instr); +#define is_corrected_ecc_error(error) (error == -EUCLEAN) +#define is_uncorrected_ecc_error(error) (error == -EBADMSG) +#define is_ecc_error(error) ((is_corrected_ecc_error(error) || \ + is_uncorrected_ecc_error(error))) + #endif /* __MTD_MTD_H__ */
These macros can be used instead of including -EUCLEAN and -EBADMSG all over the place. They should help make code a little bit more readable. Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- include/linux/mtd/mtd.h | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)