Subject: Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism

From: Marc Kleine-Budde <[email-masked]>
Date: Wed, 01 Oct 2014 12:26:57 +0200

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   |