All of lore.kernel.org
 help / color / mirror / Atom feed
From: maxime.ripard@free-electrons.com (Maxime Ripard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 05/18] power: reset: Add AT91 reset driver
Date: Tue, 8 Jul 2014 10:12:59 +0200	[thread overview]
Message-ID: <20140708081259.GM13423@lukather> (raw)
In-Reply-To: <20140708080814.GL13423@lukather>

On Tue, Jul 08, 2014 at 10:08:14AM +0200, Maxime Ripard wrote:
> On Mon, Jul 07, 2014 at 08:40:01PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > > >> move this to an assembly file more easy to read than a C code
> > > > > > 
> > > > > > Nope. It's a pain to pass variable to an external assembly file, and
> > > > > > this makes the use of global variable pretty much mandatory, which is
> > > > > > definitely not great.
> > > > > 
> > > > > Not at all I did for the PM slow clock code just write a function and pas it as a parameter
> > > > > you have 3
> > > > > 
> > > > > so basically you have to use the current and just pass at91_ramc_base[0], at91_ramc_base[1]
> > > > > and at91_rstc_base
> > > > > it?s 3 lignes of modification, if you have at91_ramc_base and at91_ramc_base same
> > > > > 
> > > > 
> > > > Yes, retrieving function parameters from assembly code is not that
> > > > complicated (the first 4 pointer values are accessible through r0-r3),
> > > > but then you'll have to store your assembly file somewhere.
> > > 
> > > Like I was saying, there's a strong preference for the inline
> > > assembly...
> > 
> > inline is horrible to read and maintain NACK
> > 
> > keep it in an assembly file it's so easy to read and follow
> > 
> > and you just have to move the file existing to the driver/power
> 
> Well, the whole rest of the kernel community feels otherwise.

Thinking a bit more about this, would using symbolic names instead of
the indices in the inline assembly work for you?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140708/d5341fb2/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Boris BREZILLON <boris.brezillon@free-electrons.com>,
	devicetree@vger.kernel.org, dbaryshkov@gmail.com,
	Nicolas FERRE <nicolas.ferre@atmel.com>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	Thomas Petazzoni <thomas@free-electrons.com>,
	Boris Brezillon <boris@free-electrons.com>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	dwmw2@infradead.org, linux@maxim.org.za,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 05/18] power: reset: Add AT91 reset driver
Date: Tue, 8 Jul 2014 10:12:59 +0200	[thread overview]
Message-ID: <20140708081259.GM13423@lukather> (raw)
In-Reply-To: <20140708080814.GL13423@lukather>

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Tue, Jul 08, 2014 at 10:08:14AM +0200, Maxime Ripard wrote:
> On Mon, Jul 07, 2014 at 08:40:01PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > > >> move this to an assembly file more easy to read than a C code
> > > > > > 
> > > > > > Nope. It's a pain to pass variable to an external assembly file, and
> > > > > > this makes the use of global variable pretty much mandatory, which is
> > > > > > definitely not great.
> > > > > 
> > > > > Not at all I did for the PM slow clock code just write a function and pas it as a parameter
> > > > > you have 3
> > > > > 
> > > > > so basically you have to use the current and just pass at91_ramc_base[0], at91_ramc_base[1]
> > > > > and at91_rstc_base
> > > > > it’s 3 lignes of modification, if you have at91_ramc_base and at91_ramc_base same
> > > > > 
> > > > 
> > > > Yes, retrieving function parameters from assembly code is not that
> > > > complicated (the first 4 pointer values are accessible through r0-r3),
> > > > but then you'll have to store your assembly file somewhere.
> > > 
> > > Like I was saying, there's a strong preference for the inline
> > > assembly...
> > 
> > inline is horrible to read and maintain NACK
> > 
> > keep it in an assembly file it's so easy to read and follow
> > 
> > and you just have to move the file existing to the driver/power
> 
> Well, the whole rest of the kernel community feels otherwise.

Thinking a bit more about this, would using symbolic names instead of
the indices in the inline assembly work for you?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2014-07-08  8:12 UTC|newest]

Thread overview: 134+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 14:14 [PATCH 00/18] AT91: cleanup of the reset and poweroff code Maxime Ripard
2014-07-03 14:14 ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 01/18] power: reset: Add if statement isntead of multiple depends on Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 02/18] AT91: setup: Switch to pr_fmt Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 03/18] AT91: G45: DT: Declare a second ram controller Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 04/18] AT91: Rework ramc mapping code Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:34   ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:34     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:34     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:53     ` Maxime Ripard
2014-07-03 14:53       ` Maxime Ripard
2014-07-03 14:53       ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 04/18] AT91: SAMA5D3: DT: Add shutdown controller Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 05/18] " Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 05/18] power: reset: Add AT91 reset driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:39   ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:39     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:39     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:59     ` Maxime Ripard
2014-07-03 14:59       ` Maxime Ripard
2014-07-03 14:59       ` Maxime Ripard
2014-07-04  2:40       ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  2:40         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  2:40         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  8:57         ` Maxime Ripard
2014-07-04  8:57           ` Maxime Ripard
2014-07-04  8:57           ` Maxime Ripard
2014-07-04  3:08       ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  3:08         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  3:08         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-04  7:14         ` Boris BREZILLON
2014-07-04  7:14           ` Boris BREZILLON
2014-07-04  7:14           ` Boris BREZILLON
2014-07-04  9:06           ` Maxime Ripard
2014-07-04  9:06             ` Maxime Ripard
2014-07-04  9:06             ` Maxime Ripard
2014-07-07 18:40             ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-07 18:40               ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-07 18:40               ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-08  8:08               ` Maxime Ripard
2014-07-08  8:08                 ` Maxime Ripard
2014-07-08  8:12                 ` Maxime Ripard [this message]
2014-07-08  8:12                   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 06/18] AT91: DT: Remove the old-style reset probing Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 06/18] power: reset: Add AT91 reset driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 07/18] AT91: DT: Remove the old-style reset probing Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 07/18] AT91: soc: Introduce register_devices callback Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 08/18] AT91: Probe the reset driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 08/18] AT91: soc: Introduce register_devices callback Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 09/18] AT91: Call at91_register_devices in the board files Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:29   ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:29     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:29     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:52     ` Maxime Ripard
2014-07-03 14:52       ` Maxime Ripard
2014-07-03 14:52       ` Maxime Ripard
2014-07-07 18:42       ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-07 18:42         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-07 18:42         ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-08  8:06         ` Maxime Ripard
2014-07-08  8:06           ` Maxime Ripard
2014-07-08  8:06           ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 09/18] AT91: Probe the reset driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 10/18] AT91: Call at91_register_devices in the board files Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 10/18] AT91: Remove reset code the machine code Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 11/18] " Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 11/18] power: reset: Add AT91 poweroff driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 12/18] AT91: DT: Remove poweroff DT probing Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 12/18] power: reset: Add AT91 poweroff driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:31   ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:31     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:31     ` Jean-Christophe PLAGNIOL-VILLARD
2014-07-03 14:53     ` Maxime Ripard
2014-07-03 14:53       ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 13/18] AT91: DT: Remove poweroff DT probing Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 13/18] AT91: Register the poweroff driver Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 14/18] " Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 14/18] AT91: Remove poweroff code Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14 ` [PATCH 15/18] AT91: pm: Remove show_reset_status function Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:14   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 15/18] AT91: Remove poweroff code Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 16/18] AT91: pm: Remove show_reset_status function Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 16/18] AT91: Remove rstc and shdwnc global base addresses Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 17/18] AT91: Remove rstc and shdwc headers Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 17/18] AT91: Remove rstc and shdwnc global base addresses Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 18/18] AT91: Remove rstc and shdwc headers Maxime Ripard
2014-07-03 14:15   ` Maxime Ripard
2014-07-03 14:15 ` [PATCH 18/18] AT91: Rework ramc mapping code Maxime Ripard
2014-07-03 14:15   ` 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=20140708081259.GM13423@lukather \
    --to=maxime.ripard@free-electrons.com \
    --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.