All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Balean <james@balean.com.au>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Add 8 and 16-bit single register pin controller support
Date: Thu,  6 Apr 2017 00:34:19 -0500	[thread overview]
Message-ID: <1491456859-11010-1-git-send-email-james@balean.com.au> (raw)
In-Reply-To: <760ef132-6b3b-b219-e5f6-00e95927dcbe@ltec.ch>

Hi All,

Thank you for your responses. I will submit a new version of the patch
with your suggestions following this.

On 1 April 2017 at 15:22, Simon Glass <sjg@chromium.org> wrote:
> Can you explain in your commit message why we want this?

Will do. I will be seeking to add TI OMAP device tree support to U-Boot
shortly, and 16-bit register widths are needed for pinmux configuration on
these platforms. Don't have an immediate need for 8-bit though, so will
remove this.

> I think ulong is better than fdt32_t, which is associated with devices
> tree.

Concur. Thanks for this suggestion.

> Instead of lots of little functions, could you have:
>
> pcs_read(ulong reg, int size)
> {
> switch (size) {
> case 8:
>    return readb(reg);
> ...

I tried to maintain consistency with mainline Linux with the separate
read functions, however it makes sense to combine the little functions.

On 2 April 2017 at 20:53, Felix Brack <fb@ltec.ch> wrote:
> I also prefer this. The corresponding switch is already there in
> 'single_configure_pins(..)', i.e. no need for an additional function.
> Using the existing function also eliminates the 'pcs_' prefix which I
> would have preferred to be 'single_' due to naming consistency
> (nitpicking, I admit).

Thanks. Yes, I will rename this to 'single_' for consistency with the
other functions (the pcs_ prefix was for consistency with Linux's
pinctrl-single driver, however it doesn't make sense to copy this
convention).

Keen for your thoughts on v2 of the patch (which I'll submit shortly) as
to me it seems cleaner to have the register read and write switches in
two separate functions, rather than in the 'single_configure_pins(..)'.


Thanks again,
James

  reply	other threads:[~2017-04-06  5:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-27  5:55 [U-Boot] [PATCH] Add 8 and 16-bit single register pin controller support James Balean
2017-04-01  4:22 ` Simon Glass
2017-04-02 10:53   ` Felix Brack
2017-04-06  5:34     ` James Balean [this message]
2017-04-06  5:38       ` [U-Boot] [PATCH v2] Add " James Balean
2017-04-07  1:44         ` James Balean
2017-04-07 13:22           ` Felix Brack
2017-04-19  2:05             ` James Balean
2017-04-07 13:16         ` Felix Brack

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=1491456859-11010-1-git-send-email-james@balean.com.au \
    --to=james@balean.com.au \
    --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.