linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Roger Quadros <rogerq@ti.com>, Wolfram Sang <wsa@the-dreams.de>
Cc: wg@grandegger.com, tony@atomide.com, tglx@linutronix.de,
	mugunthanvnm@ti.com, george.cherian@ti.com, balbi@ti.com,
	nsekhar@ti.comnm@ti.com, sergei.shtylyov@cogentembedded.com,
	linux-omap@vger.kernel.org, linux-can@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism
Date: Wed, 01 Oct 2014 12:26:57 +0200	[thread overview]
Message-ID: <542BD6F1.2020900@pengutronix.de> (raw)
In-Reply-To: <542BD3A7.5060200@ti.com>

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

On 10/01/2014 12:12 PM, Roger Quadros wrote:
> On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote:
>> On 10/01/2014 11:06 AM, Roger Quadros wrote:
>>> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote:
>>>> On 10/01/2014 10:45 AM, Roger Quadros wrote:
>>>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote:
>>>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote:
>>>>>>>
>>>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for
>>>>>>>> additional parameters. Have a look at:
>>>>>>>>
>>>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>>>>>>>
>>>>>>>> First get the regmap, then the 1st argument is the offset in the regmap,
>>>>>>>> the 2nd and 3rd could be the bits.
>>>>>>>
>>>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit>
>>>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift>
>>>>>>
>>>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc.
>>>>>>
>>>>>> Roger, can we derive both start and done bit from a common reg_shift?
>>>>>
>>>>> I'm sorry I didn't understand what you meant.
>>>>>
>>>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us.
>>>>> Even though reg offset is the same for both the DCAN instances.
>>>>
>>>> What's start bit and stop bit for instance 0 and 1 on that SoC that has
>>>> two instances?
>>>>
>>>
>>> we have 3 SoCs at the moment, all have 2 DCAN instances.
>>>
>>> AM33xx & AM43xx
>>> instance	start	stop
>>> 1		0	8
>>> 2		1	9	
>>
>> If we use a 0-based numbering for the instances:
>> instance	start		stop
>> 0		(0 << instance)	(8 << instance)
>> 1		(0 << instance)	(8 << instance)
>>
> 
> How does the instance number get set? What happens on boards where
> the first instance is unused while the second one is in use?

Via a single "instance" parameter of the syscon phandle....

> 
>>> DRA7xx
>>> instance	start	stop
>>> 1		3	1
>>> 2		5	2
>>                 ^
>> 5 or 4?
> 
> Unfortunately it is 5 ;)
> We have display IP related bit in between 3 and 5 :P

What on earth were the HW engineers thinking????????????

...if we just have the instance parameter in the syscon phandle, we have
to put the mapping into the driver, which makes IMHO no sense, because
you have to touch the driver, if there is another SoC with the DCAN core.

AFAICS we have these options:
1) syscon phandle with three parameters:
   reg offset, start bit shift, stop bit shift
   (the name of the syscon phandle is a different topic)
2) a single ti,start-stop-bit option with two parameter
3) two ti,* entries with a single parameter each
   (as in Roger's initial patch)

Wolfram, which solution do you prefer? I'm in favour of 3 (+ plus a
phandle with a reg offset parameter).

puzzled,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  reply	other threads:[~2014-10-01 10:26 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-09 14:31 [PATCH v2 0/3] net: can: Use syscon regmap for TI specific RAMINIT register Roger Quadros
2014-09-09 14:31 ` [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout Roger Quadros
2014-09-09 14:34   ` Marc Kleine-Budde
2014-09-09 14:39     ` Roger Quadros
2014-09-16 14:13       ` Marc Kleine-Budde
2014-09-09 14:34   ` Nishanth Menon
2014-09-09 14:45     ` Roger Quadros
2014-09-09 14:51       ` Nishanth Menon
2014-09-09 14:54         ` Roger Quadros
2014-09-09 14:31 ` [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism Roger Quadros
2014-09-30 13:26   ` Wolfram Sang
2014-09-30 13:33     ` Roger Quadros
2014-09-30 13:52       ` Wolfram Sang
2014-09-30 13:58         ` Roger Quadros
2014-09-30 14:19           ` Wolfram Sang
2014-09-30 14:22             ` Marc Kleine-Budde
2014-09-30 14:49               ` Wolfram Sang
2014-09-30 15:01                 ` Marc Kleine-Budde
2014-09-30 15:25                   ` Wolfram Sang
2014-09-30 16:04                     ` Marc Kleine-Budde
2014-10-01  8:45                       ` Roger Quadros
2014-10-01  8:47                         ` Marc Kleine-Budde
2014-10-01  9:06                           ` Roger Quadros
2014-10-01 10:01                             ` Marc Kleine-Budde
2014-10-01 10:12                               ` Roger Quadros
2014-10-01 10:26                                 ` Marc Kleine-Budde [this message]
2014-10-01 10:43                                   ` Wolfram Sang
2014-10-01 10:57                                     ` Roger Quadros
2014-10-01 11:06                                       ` Marc Kleine-Budde
2014-10-01 11:10                                       ` Wolfram Sang
2014-10-01 11:11                                     ` Marc Kleine-Budde
2014-09-30 13:45     ` Marc Kleine-Budde
2014-09-30 14:02       ` Roger Quadros
2014-09-30 14:11         ` Marc Kleine-Budde
2014-09-09 14:31 ` [PATCH v2 3/3] net: can: c_can: Add support for START pulse in RAMINIT sequence Roger Quadros

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=542BD6F1.2020900@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=balbi@ti.com \
    --cc=george.cherian@ti.com \
    --cc=mugunthanvnm@ti.com \
    --cc=nsekhar@ti.comnm \
    --cc=rogerq@ti.com \
    --cc=tglx@linutronix.de \
    --cc=tony@atomide.com \
    --cc=wg@grandegger.com \
    --cc=wsa@the-dreams.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).