All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Kocialkowski <contact@paulk.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] fdt: Pass the device serial number through devicetree
Date: Sun, 24 May 2015 12:03:21 +0200	[thread overview]
Message-ID: <1432461801.2549.6.camel@collins> (raw)
In-Reply-To: <CAPnjgZ11CmHF2=TFvEZB6mp0tVEXiMi6uoYpiPtxTzxWPGWk+A@mail.gmail.com>

Hi Simon,

[?]

> > > > diff --git a/include/fdt_support.h b/include/fdt_support.h
> > > > index 5d4f28d..56185c9 100644
> > > > --- a/include/fdt_support.h
> > > > +++ b/include/fdt_support.h
> > > > @@ -16,6 +16,7 @@ u32 fdt_getprop_u32_default_node(const void *fdt, int off, int cell,
> > > >                                 const char *prop, const u32 dflt);
> > > >  u32 fdt_getprop_u32_default(const void *fdt, const char *path,
> > > >                                 const char *prop, const u32 dflt);
> > > > +int fdt_root(void *fdt);
> > >
> > > Please can you add a comment for this in the standard style?
> >
> > As far as I can see, fdt_root plays a similar role to fdt_chosen and
> > fdt_initrd, all of which are defined in fdt_support.c and used in
> > image-fdt.c's image_setup_libfdt.
> >
> > Their prototypes are defined in fdt_support.h and neither fdt_chosen nor
> > fdt_initrd have such a comment, so I didn't think it was very consistent
> > to add one for fdt_root when writing the patch.
> >
> > Now if you think it's worth adding such a comment for the sake of
> > documentation, I don't object to it but it still leaves me with a
> > feeling of inconsistency with regard to other similar prototypes.
> 
> You want uncommented code for consistency?? :-)
> 
> If you are concerned, you could add a new patch that adds comments for
> some other functions in fdt_support.h. That would be welcome. But at
> least, please add comments for new code.

Fair enough, I've just sent out another patch that adds (minimalistic)
documentation for all 3 functions.

> The problem with that is that no one is going to go through and
> comment all these APIs. Generally the only way the code quality
> improves is when people add new features or refactor the code for some
> reason.

I understand.

Thanks for the review!

> > > >  int fdt_chosen(void *fdt);
> > > >  int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end);
> > > >  void do_fixup_by_path(void *fdt, const char *path, const char *prop,
> > > > --
> > > > 1.9.1
> > > >
> 
> Regards,
> Simon

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150524/9fccb252/attachment.sig>

      reply	other threads:[~2015-05-24 10:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-21  9:27 [U-Boot] [PATCH v2] fdt: Pass the device serial number through devicetree Paul Kocialkowski
2015-05-21 20:32 ` Simon Glass
2015-05-22 10:55   ` Paul Kocialkowski
2015-05-22 16:12     ` Simon Glass
2015-05-24 10:03       ` Paul Kocialkowski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1432461801.2549.6.camel@collins \
    --to=contact@paulk.fr \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.