All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Antoine Ténart" <antoine.tenart@free-electrons.com>
To: Tejun Heo <tj@kernel.org>
Cc: "Antoine Ténart" <antoine.tenart@free-electrons.com>,
	sebastian.hesselbarth@gmail.com, kishon@ti.com,
	alexandre.belloni@free-electrons.com,
	thomas.petazzoni@free-electrons.com, zmxu@marvell.com,
	jszhang@marvell.com, linux-arm-kernel@lists.infradead.org,
	linux-ide@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v11 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure
Date: Wed, 30 Jul 2014 10:20:38 +0200	[thread overview]
Message-ID: <20140730082038.GB29900@kwain> (raw)
In-Reply-To: <20140729144042.GD4791@htj.dyndns.org>

On Tue, Jul 29, 2014 at 10:40:42AM -0400, Tejun Heo wrote:
> On Thu, Jul 24, 2014 at 11:17:25AM +0200, Antoine Ténart wrote:
> > @@ -321,6 +321,8 @@ struct ahci_host_priv {
> >  	u32			cap;		/* cap to use */
> >  	u32			cap2;		/* cap2 to use */
> >  	u32			port_map;	/* port map to use */
> > +	u32			force_port_map;	/* force port map */
> > +	u32			mask_port_map;	/* mask out particular bits */
> 
> Let's collect the inputs, including flags, at the top and mark them clearly.

Done.

> 
> >  int ahci_platform_init_host(struct platform_device *pdev,
> >  			    struct ahci_host_priv *hpriv,
> >  			    const struct ata_port_info *pi_template,
> > -			    unsigned long host_flags,
> > -			    unsigned int force_port_map,
> > -			    unsigned int mask_port_map)
> > +			    unsigned long host_flags)
> 
> This doesn't make much sense to me.  Near the head of the function, it
> does
> 
> 	hpriv->flags |= host_flags;
> 
> Wouldn't it make more sense to just let the caller set hpriv->flags?

I just removed the host_flags parameter and updated the drivers calling
it.


How do you want me to send the series? There is two conflicts when
applying to libata/for-3.17:
- patch 4/8: it takes into account a patch not in libata/for-3.17 but
  added before rc7. It should be better to first merge rc7, otherwise
  some modifications won't make sense.
- patch 6/8: "fsl,imx53-ahci" was removed from the documentation in
  libata/for-3.17 but not in rc7. Resolving the conflict is really
  simple.

I think it's better to apply the whole thing after merging rc7 into
libata/for-3.17, because it will only have a really simple conflict to
resolve.

Please tell me what do you prefer before I send the updated version.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: antoine.tenart@free-electrons.com (Antoine Ténart)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v11 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure
Date: Wed, 30 Jul 2014 10:20:38 +0200	[thread overview]
Message-ID: <20140730082038.GB29900@kwain> (raw)
In-Reply-To: <20140729144042.GD4791@htj.dyndns.org>

On Tue, Jul 29, 2014 at 10:40:42AM -0400, Tejun Heo wrote:
> On Thu, Jul 24, 2014 at 11:17:25AM +0200, Antoine T?nart wrote:
> > @@ -321,6 +321,8 @@ struct ahci_host_priv {
> >  	u32			cap;		/* cap to use */
> >  	u32			cap2;		/* cap2 to use */
> >  	u32			port_map;	/* port map to use */
> > +	u32			force_port_map;	/* force port map */
> > +	u32			mask_port_map;	/* mask out particular bits */
> 
> Let's collect the inputs, including flags, at the top and mark them clearly.

Done.

> 
> >  int ahci_platform_init_host(struct platform_device *pdev,
> >  			    struct ahci_host_priv *hpriv,
> >  			    const struct ata_port_info *pi_template,
> > -			    unsigned long host_flags,
> > -			    unsigned int force_port_map,
> > -			    unsigned int mask_port_map)
> > +			    unsigned long host_flags)
> 
> This doesn't make much sense to me.  Near the head of the function, it
> does
> 
> 	hpriv->flags |= host_flags;
> 
> Wouldn't it make more sense to just let the caller set hpriv->flags?

I just removed the host_flags parameter and updated the drivers calling
it.


How do you want me to send the series? There is two conflicts when
applying to libata/for-3.17:
- patch 4/8: it takes into account a patch not in libata/for-3.17 but
  added before rc7. It should be better to first merge rc7, otherwise
  some modifications won't make sense.
- patch 6/8: "fsl,imx53-ahci" was removed from the documentation in
  libata/for-3.17 but not in rc7. Resolving the conflict is really
  simple.

I think it's better to apply the whole thing after merging rc7 into
libata/for-3.17, because it will only have a really simple conflict to
resolve.

Please tell me what do you prefer before I send the updated version.

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2014-07-30  8:20 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-24  9:17 [PATCH v11 0/8] ARM: berlin: add AHCI support Antoine Ténart
2014-07-24  9:17 ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 1/8] phy: add a driver for the Berlin SATA PHY Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 2/8] Documentation: bindings: add " Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 3/8] ata: libahci_platform: move port_map parameters into the AHCI structure Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-29 14:40   ` Tejun Heo
2014-07-29 14:40     ` Tejun Heo
2014-07-30  8:20     ` Antoine Ténart [this message]
2014-07-30  8:20       ` Antoine Ténart
2014-07-30 15:35       ` Tejun Heo
2014-07-30 15:35         ` Tejun Heo
2014-07-30 16:47         ` Antoine Ténart
2014-07-30 16:47           ` Antoine Ténart
2014-07-30 16:50           ` Tejun Heo
2014-07-30 16:50             ` Tejun Heo
     [not found] ` <1406193450-17283-1-git-send-email-antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-07-24  9:17   ` [PATCH v11 4/8] ata: libahci: allow to use multiple PHYs Antoine Ténart
2014-07-24  9:17     ` Antoine Ténart
2014-07-24  9:17     ` Antoine Ténart
2014-07-28 10:29     ` Hans de Goede
2014-07-28 10:29       ` Hans de Goede
2014-07-28 17:27       ` Tejun Heo
2014-07-28 17:27         ` Tejun Heo
2014-07-29  7:14         ` Antoine Ténart
2014-07-29  7:14           ` Antoine Ténart
     [not found]     ` <1406193450-17283-5-git-send-email-antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-07-30  9:12       ` Kishon Vijay Abraham I
2014-07-30  9:12         ` Kishon Vijay Abraham I
2014-07-30  9:12         ` Kishon Vijay Abraham I
2014-07-24  9:17 ` [PATCH v11 5/8] ata: ahci_platform: add a generic AHCI compatible Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 6/8] Documentation: bindings: document the sub-nodes AHCI bindings Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 7/8] ARM: berlin: add the AHCI node for the BG2Q Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart
2014-07-24  9:17 ` [PATCH v11 8/8] ARM: berlin: enable the eSATA interface on the BG2Q DMP Antoine Ténart
2014-07-24  9:17   ` Antoine Ténart

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=20140730082038.GB29900@kwain \
    --to=antoine.tenart@free-electrons.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jszhang@marvell.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=tj@kernel.org \
    --cc=zmxu@marvell.com \
    /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.