All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 2/8] ARM: sunxi: Split out the various machines into separate files
Date: Wed, 23 Apr 2014 15:33:29 +0200	[thread overview]
Message-ID: <6249453.RCSRl1Jr3F@wuerfel> (raw)
In-Reply-To: <20140423132829.GT24905@lukather>

On Wednesday 23 April 2014 15:28:29 Maxime Ripard wrote:
> On Wed, Apr 23, 2014 at 02:33:50PM +0200, Arnd Bergmann wrote:
> > On Thursday 17 April 2014, Maxime Ripard wrote:
> > > This will allow to add per-SoC hooks more easily.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >  arch/arm/mach-sunxi/Makefile  |   6 +-
> > >  arch/arm/mach-sunxi/restart.c | 104 +++++++++++++++++++++++++++
> > >  arch/arm/mach-sunxi/restart.h |  20 ++++++
> > >  arch/arm/mach-sunxi/sun4i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sun5i.c   |  37 ++++++++++
> > >  arch/arm/mach-sunxi/sun6i.c   |  49 +++++++++++++
> > >  arch/arm/mach-sunxi/sun7i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sunxi.c   | 164 ------------------------------------------
> > >  8 files changed, 287 insertions(+), 165 deletions(-)
> > 
> > This doesn't look like a move in the right direction, and I don't see
> > the connection with the DMA driver.
> 
> This is actually not adding any code at all, just reorganizing it.
> 
> This was also something that would allow to have per-family
> configuration options, and hence not select drivers like the pinctrl
> one that are pretty huge if they're not needed.

I don't mind having split Kconfig entries to control which drivers
are used. It was more about the absolute growth in C source code
lines above.

> > Splitting out the restart code is good, but please complete this by
> > moving it into a driver directory. My preferred location would be
> > within the watchdog driver, since it's really the same registers.
> > You can turn on the watchdog driver as built-in in the defconfig.
> > While it's not ideal that you can end up without a restart function
> > if the driver does not get loaded, I would still prefer that over
> > having the driver in the mach-sunxi directory.
> 
> Ok. The thing is, we don't have any watchdog driver for the A31 for
> the moment, so the restart code will have to stay there. But I can
> totally do that for the A10/A20.
> 
> > An alternative would be to move the restart code into
> > drivers/power/reset.
> 
> That would be a good place for the A31 code then.

Ok, good.

> > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > deleted file mode 100644
> > > index 460b5a4962ef..000000000000
> > > --- a/arch/arm/mach-sunxi/sunxi.c
> > 
> > Instead of splitting up this file, I think it would be better to
> > reduce the number of special hacks required per machine. Note that
> > for arm64 we require platforms not to have any code outside of
> > the drivers directory, and I'd like to get to the same situation
> > for much of arch/arm/mach-* as well.
> 
> I think it's fair.
> 
> How would you enforce default clock parenting and/or rate then?

See my other reply.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: Maxime Ripard
	<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Emilio Lopez <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org>,
	Dan Williams
	<dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Mike Turquette
	<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	andriy.shevchenko-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org,
	shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org,
	dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v6 2/8] ARM: sunxi: Split out the various machines into separate files
Date: Wed, 23 Apr 2014 15:33:29 +0200	[thread overview]
Message-ID: <6249453.RCSRl1Jr3F@wuerfel> (raw)
In-Reply-To: <20140423132829.GT24905@lukather>

On Wednesday 23 April 2014 15:28:29 Maxime Ripard wrote:
> On Wed, Apr 23, 2014 at 02:33:50PM +0200, Arnd Bergmann wrote:
> > On Thursday 17 April 2014, Maxime Ripard wrote:
> > > This will allow to add per-SoC hooks more easily.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > ---
> > >  arch/arm/mach-sunxi/Makefile  |   6 +-
> > >  arch/arm/mach-sunxi/restart.c | 104 +++++++++++++++++++++++++++
> > >  arch/arm/mach-sunxi/restart.h |  20 ++++++
> > >  arch/arm/mach-sunxi/sun4i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sun5i.c   |  37 ++++++++++
> > >  arch/arm/mach-sunxi/sun6i.c   |  49 +++++++++++++
> > >  arch/arm/mach-sunxi/sun7i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sunxi.c   | 164 ------------------------------------------
> > >  8 files changed, 287 insertions(+), 165 deletions(-)
> > 
> > This doesn't look like a move in the right direction, and I don't see
> > the connection with the DMA driver.
> 
> This is actually not adding any code at all, just reorganizing it.
> 
> This was also something that would allow to have per-family
> configuration options, and hence not select drivers like the pinctrl
> one that are pretty huge if they're not needed.

I don't mind having split Kconfig entries to control which drivers
are used. It was more about the absolute growth in C source code
lines above.

> > Splitting out the restart code is good, but please complete this by
> > moving it into a driver directory. My preferred location would be
> > within the watchdog driver, since it's really the same registers.
> > You can turn on the watchdog driver as built-in in the defconfig.
> > While it's not ideal that you can end up without a restart function
> > if the driver does not get loaded, I would still prefer that over
> > having the driver in the mach-sunxi directory.
> 
> Ok. The thing is, we don't have any watchdog driver for the A31 for
> the moment, so the restart code will have to stay there. But I can
> totally do that for the A10/A20.
> 
> > An alternative would be to move the restart code into
> > drivers/power/reset.
> 
> That would be a good place for the A31 code then.

Ok, good.

> > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > deleted file mode 100644
> > > index 460b5a4962ef..000000000000
> > > --- a/arch/arm/mach-sunxi/sunxi.c
> > 
> > Instead of splitting up this file, I think it would be better to
> > reduce the number of special hacks required per machine. Note that
> > for arm64 we require platforms not to have any code outside of
> > the drivers directory, and I'd like to get to the same situation
> > for much of arch/arm/mach-* as well.
> 
> I think it's fair.
> 
> How would you enforce default clock parenting and/or rate then?

See my other reply.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Emilio Lopez <emilio@elopez.com.ar>,
	Dan Williams <dan.j.williams@intel.com>,
	Vinod Koul <vinod.koul@intel.com>,
	devicetree@vger.kernel.org,
	Mike Turquette <mturquette@linaro.org>,
	andriy.shevchenko@intel.com, linux-kernel@vger.kernel.org,
	zhuzhenhua@allwinnertech.com, shuge@allwinnertech.com,
	linux-sunxi@googlegroups.com, kevin.z.m.zh@gmail.com,
	sunny@allwinnertech.com, dmaengine@vger.kernel.org
Subject: Re: [PATCH v6 2/8] ARM: sunxi: Split out the various machines into separate files
Date: Wed, 23 Apr 2014 15:33:29 +0200	[thread overview]
Message-ID: <6249453.RCSRl1Jr3F@wuerfel> (raw)
In-Reply-To: <20140423132829.GT24905@lukather>

On Wednesday 23 April 2014 15:28:29 Maxime Ripard wrote:
> On Wed, Apr 23, 2014 at 02:33:50PM +0200, Arnd Bergmann wrote:
> > On Thursday 17 April 2014, Maxime Ripard wrote:
> > > This will allow to add per-SoC hooks more easily.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >  arch/arm/mach-sunxi/Makefile  |   6 +-
> > >  arch/arm/mach-sunxi/restart.c | 104 +++++++++++++++++++++++++++
> > >  arch/arm/mach-sunxi/restart.h |  20 ++++++
> > >  arch/arm/mach-sunxi/sun4i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sun5i.c   |  37 ++++++++++
> > >  arch/arm/mach-sunxi/sun6i.c   |  49 +++++++++++++
> > >  arch/arm/mach-sunxi/sun7i.c   |  36 ++++++++++
> > >  arch/arm/mach-sunxi/sunxi.c   | 164 ------------------------------------------
> > >  8 files changed, 287 insertions(+), 165 deletions(-)
> > 
> > This doesn't look like a move in the right direction, and I don't see
> > the connection with the DMA driver.
> 
> This is actually not adding any code at all, just reorganizing it.
> 
> This was also something that would allow to have per-family
> configuration options, and hence not select drivers like the pinctrl
> one that are pretty huge if they're not needed.

I don't mind having split Kconfig entries to control which drivers
are used. It was more about the absolute growth in C source code
lines above.

> > Splitting out the restart code is good, but please complete this by
> > moving it into a driver directory. My preferred location would be
> > within the watchdog driver, since it's really the same registers.
> > You can turn on the watchdog driver as built-in in the defconfig.
> > While it's not ideal that you can end up without a restart function
> > if the driver does not get loaded, I would still prefer that over
> > having the driver in the mach-sunxi directory.
> 
> Ok. The thing is, we don't have any watchdog driver for the A31 for
> the moment, so the restart code will have to stay there. But I can
> totally do that for the A10/A20.
> 
> > An alternative would be to move the restart code into
> > drivers/power/reset.
> 
> That would be a good place for the A31 code then.

Ok, good.

> > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > deleted file mode 100644
> > > index 460b5a4962ef..000000000000
> > > --- a/arch/arm/mach-sunxi/sunxi.c
> > 
> > Instead of splitting up this file, I think it would be better to
> > reduce the number of special hacks required per machine. Note that
> > for arm64 we require platforms not to have any code outside of
> > the drivers directory, and I'd like to get to the same situation
> > for much of arch/arm/mach-* as well.
> 
> I think it's fair.
> 
> How would you enforce default clock parenting and/or rate then?

See my other reply.

	Arnd

  reply	other threads:[~2014-04-23 13:33 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-17  8:46 [PATCH v6 0/8] Add support for the Allwinner A31 DMA Controller Maxime Ripard
2014-04-17  8:46 ` Maxime Ripard
2014-04-17  8:46 ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 1/8] clk: sunxi: Remove calls to clk_put Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 2/8] ARM: sunxi: Split out the various machines into separate files Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-23 12:33   ` Arnd Bergmann
2014-04-23 12:33     ` Arnd Bergmann
2014-04-23 12:33     ` Arnd Bergmann
2014-04-23 13:28     ` Maxime Ripard
2014-04-23 13:28       ` Maxime Ripard
2014-04-23 13:28       ` Maxime Ripard
2014-04-23 13:33       ` Arnd Bergmann [this message]
2014-04-23 13:33         ` Arnd Bergmann
2014-04-23 13:33         ` Arnd Bergmann
2014-04-17  8:46 ` [PATCH v6 3/8] ARM: sunxi: Move the clock protection to machine hooks Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-23 12:39   ` Arnd Bergmann
2014-04-23 12:39     ` Arnd Bergmann
2014-04-23 13:17     ` Maxime Ripard
2014-04-23 13:17       ` Maxime Ripard
2014-04-23 13:17       ` Maxime Ripard
2014-04-23 13:31       ` Arnd Bergmann
2014-04-23 13:31         ` Arnd Bergmann
2014-04-23 13:31         ` Arnd Bergmann
2014-05-13  1:30         ` Mike Turquette
2014-05-13  1:30           ` Mike Turquette
2014-05-13  1:30           ` Mike Turquette
2014-04-17  8:46 ` [PATCH v6 4/8] ARM: sun6i: Protect CPU clock Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 5/8] ARM: sun6i: Protect SDRAM gating bit Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 6/8] ARM: sun6i: Sort the NMI node by physical address Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-23 12:39   ` Arnd Bergmann
2014-04-23 12:39     ` Arnd Bergmann
2014-04-23 12:39     ` Arnd Bergmann
2014-04-23 13:11     ` Maxime Ripard
2014-04-23 13:11       ` Maxime Ripard
2014-04-23 13:11       ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 7/8] DMA: sun6i: Add driver for the Allwinner A31 DMA controller Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46 ` [PATCH v6 8/8] ARM: sun6i: dt: Add A31 DMA controller to DTSI Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard
2014-04-17  8:46   ` Maxime Ripard

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=6249453.RCSRl1Jr3F@wuerfel \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.