diff mbox

[U-Boot,v2,1/6] spi: cadence_qspi: move trigger base configuration in init

Message ID 1437013654-29387-2-git-send-email-vikas.manocha@st.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Vikas MANOCHA July 16, 2015, 2:27 a.m. UTC 
Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changes in v2: Rebased to master

 drivers/spi/cadence_qspi_apb.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Marek Vasut Aug. 13, 2015, 2:07 a.m. UTC | #1 
On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:

Commit message is missing.

> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> ---
> 
> Changes in v2: Rebased to master
> 
>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> cadence_spi_platdata *plat)
> 
>  	/* Indirect mode configurations */
>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),

You can drop the parenthesis around the first argument, they're useless.
Also, the indent of the second arg should be fixed, I believe checkpatch
might even complain about it.

> +				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> 
>  	/* Disable all interrupts */
>  	writel(0, plat->regbase + CQSPI_REG_IRQMASK);
> @@ -693,10 +695,6 @@ int cadence_qspi_apb_indirect_read_setup(struct
> cadence_spi_platdata *plat, /* for normal read (only ramtron as of now) */
>  		addr_bytes = cmdlen - 1;
> 
> -	/* Setup the indirect trigger address */
> -	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> -	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> -
>  	/* Configure the opcode */
>  	rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
> 
> @@ -790,9 +788,6 @@ int cadence_qspi_apb_indirect_write_setup(struct
> cadence_spi_platdata *plat, cmdlen, (unsigned int)cmdbuf);
>  		return -EINVAL;
>  	}
> -	/* Setup the indirect trigger address */
> -	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> -	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> 
>  	/* Configure the opcode */
>  	reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB;

Best regards,
Marek Vasut
Vikas MANOCHA Aug. 13, 2015, 3:50 p.m. UTC | #2 
Hi Marek,

On 08/12/2015 07:07 PM, Marek Vasut wrote:
> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
> 
> Commit message is missing.

Actually subject of the mail was sufficient, this patch just moves the register configuration in init.

> 
>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>> ---
>>
>> Changes in v2: Rebased to master
>>
>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/spi/cadence_qspi_apb.c
>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
>> cadence_spi_platdata *plat)
>>
>>  	/* Indirect mode configurations */
>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> 
> You can drop the parenthesis around the first argument, they're useless.
> Also, the indent of the second arg should be fixed, I believe checkpatch
> might even complain about it.

ok.

Rgds,
Vikas

> 
>> +				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
>>
>>  	/* Disable all interrupts */
>>  	writel(0, plat->regbase + CQSPI_REG_IRQMASK);
>> @@ -693,10 +695,6 @@ int cadence_qspi_apb_indirect_read_setup(struct
>> cadence_spi_platdata *plat, /* for normal read (only ramtron as of now) */
>>  		addr_bytes = cmdlen - 1;
>>
>> -	/* Setup the indirect trigger address */
>> -	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
>> -	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
>> -
>>  	/* Configure the opcode */
>>  	rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
>>
>> @@ -790,9 +788,6 @@ int cadence_qspi_apb_indirect_write_setup(struct
>> cadence_spi_platdata *plat, cmdlen, (unsigned int)cmdbuf);
>>  		return -EINVAL;
>>  	}
>> -	/* Setup the indirect trigger address */
>> -	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
>> -	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
>>
>>  	/* Configure the opcode */
>>  	reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB;
> 
> Best regards,
> Marek Vasut
> .
>
Marek Vasut Aug. 13, 2015, 5:35 p.m. UTC | #3 
On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
> Hi Marek,

Hi!

> On 08/12/2015 07:07 PM, Marek Vasut wrote:
> > On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
> > 
> > Commit message is missing.
> 
> Actually subject of the mail was sufficient, this patch just moves the
> register configuration in init.

NAK, fix the commit message.

> >> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >> ---
> >> 
> >> Changes in v2: Rebased to master
> >> 
> >>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
> >>  1 file changed, 2 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/spi/cadence_qspi_apb.c
> >> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
> >> --- a/drivers/spi/cadence_qspi_apb.c
> >> +++ b/drivers/spi/cadence_qspi_apb.c
> >> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> >> cadence_spi_platdata *plat)
> >> 
> >>  	/* Indirect mode configurations */
> >>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
> >> 
> >> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> > 
> > You can drop the parenthesis around the first argument, they're useless.
> > Also, the indent of the second arg should be fixed, I believe checkpatch
> > might even complain about it.
> 
> ok.

Thanks :)

Best regards,
Marek Vasut
Vikas MANOCHA Aug. 13, 2015, 7:05 p.m. UTC | #4 
Hi,

On 08/13/2015 10:35 AM, Marek Vasut wrote:
> On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
>> Hi Marek,
> 
> Hi!
> 
>> On 08/12/2015 07:07 PM, Marek Vasut wrote:
>>> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
>>>
>>> Commit message is missing.
>>
>> Actually subject of the mail was sufficient, this patch just moves the
>> register configuration in init.
> 
> NAK, fix the commit message.

ok.

Rgds,
Vikas

> 
>>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>>>> ---
>>>>
>>>> Changes in v2: Rebased to master
>>>>
>>>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/cadence_qspi_apb.c
>>>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
>>>> --- a/drivers/spi/cadence_qspi_apb.c
>>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
>>>> cadence_spi_platdata *plat)
>>>>
>>>>  	/* Indirect mode configurations */
>>>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>>>>
>>>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
>>>
>>> You can drop the parenthesis around the first argument, they're useless.
>>> Also, the indent of the second arg should be fixed, I believe checkpatch
>>> might even complain about it.
>>
>> ok.
> 
> Thanks :)
> 
> Best regards,
> Marek Vasut
> .
>
Vikas MANOCHA Aug. 14, 2015, 1:24 a.m. UTC | #5 
Hi Marek,

On 08/13/2015 10:35 AM, Marek Vasut wrote:
> On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
>> Hi Marek,
> 
> Hi!
> 
>> On 08/12/2015 07:07 PM, Marek Vasut wrote:
>>> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
>>>
>>> Commit message is missing.
>>
>> Actually subject of the mail was sufficient, this patch just moves the
>> register configuration in init.
> 
> NAK, fix the commit message.
> 
>>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>>>> ---
>>>>
>>>> Changes in v2: Rebased to master
>>>>
>>>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/cadence_qspi_apb.c
>>>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
>>>> --- a/drivers/spi/cadence_qspi_apb.c
>>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
>>>> cadence_spi_platdata *plat)
>>>>
>>>>  	/* Indirect mode configurations */
>>>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>>>>
>>>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
>>>
>>> You can drop the parenthesis around the first argument, they're useless.
>>> Also, the indent of the second arg should be fixed, I believe checkpatch
>>> might even complain about it.

ok for first comment about parenthesis but indent of second arg seems ok.
yes, checkpatch warning was "CHECK: Alignment should match open parenthesis" but i ignored it.
To respect 80 column, i had to move second arg in another line. Am i missing something ?

Rgds,
Vikas

>>
>> ok.
> 
> Thanks :)
> 
> Best regards,
> Marek Vasut
> .
>
Marek Vasut Aug. 14, 2015, 1:43 a.m. UTC | #6 
On Friday, August 14, 2015 at 03:24:10 AM, vikas wrote:
> Hi Marek,

Hi,

> On 08/13/2015 10:35 AM, Marek Vasut wrote:
> > On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
> >> Hi Marek,
> > 
> > Hi!
> > 
> >> On 08/12/2015 07:07 PM, Marek Vasut wrote:
> >>> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
> >>> 
> >>> Commit message is missing.
> >> 
> >> Actually subject of the mail was sufficient, this patch just moves the
> >> register configuration in init.
> > 
> > NAK, fix the commit message.
> > 
> >>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >>>> ---
> >>>> 
> >>>> Changes in v2: Rebased to master
> >>>> 
> >>>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
> >>>>  1 file changed, 2 insertions(+), 7 deletions(-)
> >>>> 
> >>>> diff --git a/drivers/spi/cadence_qspi_apb.c
> >>>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
> >>>> --- a/drivers/spi/cadence_qspi_apb.c
> >>>> +++ b/drivers/spi/cadence_qspi_apb.c
> >>>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> >>>> cadence_spi_platdata *plat)
> >>>> 
> >>>>  	/* Indirect mode configurations */
> >>>>  	writel((plat->sram_size/2), plat->regbase +
> >>>>  	CQSPI_REG_SRAMPARTITION);
> >>>> 
> >>>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> >>> 
> >>> You can drop the parenthesis around the first argument, they're
> >>> useless. Also, the indent of the second arg should be fixed, I believe
> >>> checkpatch might even complain about it.
> 
> ok for first comment about parenthesis but indent of second arg seems ok.
> yes, checkpatch warning was "CHECK: Alignment should match open
> parenthesis" but i ignored it. To respect 80 column, i had to move second
> arg in another line. Am i missing something ?

Just don't ignore the checkpatch warnings next time please ;-)

Best regards,
Marek Vasut
Vikas MANOCHA Aug. 14, 2015, 1:44 a.m. UTC | #7 
Hi,

On 08/13/2015 06:43 PM, Marek Vasut wrote:
> On Friday, August 14, 2015 at 03:24:10 AM, vikas wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 08/13/2015 10:35 AM, Marek Vasut wrote:
>>> On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
>>>> Hi Marek,
>>>
>>> Hi!
>>>
>>>> On 08/12/2015 07:07 PM, Marek Vasut wrote:
>>>>> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
>>>>>
>>>>> Commit message is missing.
>>>>
>>>> Actually subject of the mail was sufficient, this patch just moves the
>>>> register configuration in init.
>>>
>>> NAK, fix the commit message.
>>>
>>>>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2: Rebased to master
>>>>>>
>>>>>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>>>>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/spi/cadence_qspi_apb.c
>>>>>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
>>>>>> --- a/drivers/spi/cadence_qspi_apb.c
>>>>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>>>>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
>>>>>> cadence_spi_platdata *plat)
>>>>>>
>>>>>>  	/* Indirect mode configurations */
>>>>>>  	writel((plat->sram_size/2), plat->regbase +
>>>>>>  	CQSPI_REG_SRAMPARTITION);
>>>>>>
>>>>>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
>>>>>
>>>>> You can drop the parenthesis around the first argument, they're
>>>>> useless. Also, the indent of the second arg should be fixed, I believe
>>>>> checkpatch might even complain about it.
>>
>> ok for first comment about parenthesis but indent of second arg seems ok.
>> yes, checkpatch warning was "CHECK: Alignment should match open
>> parenthesis" but i ignored it. To respect 80 column, i had to move second
>> arg in another line. Am i missing something ?
> 
> Just don't ignore the checkpatch warnings next time please ;-)

This CHECK message is gonna come in any case if i move second argument in second line to restrict 80 column rule.
My understanding is to ignore this CHECK message.

Rgds,
Vikas

> 
> Best regards,
> Marek Vasut
> .
>
Marek Vasut Aug. 14, 2015, 1:55 a.m. UTC | #8 
On Friday, August 14, 2015 at 03:44:41 AM, vikas wrote:
> Hi,
> 
> On 08/13/2015 06:43 PM, Marek Vasut wrote:
> > On Friday, August 14, 2015 at 03:24:10 AM, vikas wrote:
> >> Hi Marek,
> > 
> > Hi,
> > 
> >> On 08/13/2015 10:35 AM, Marek Vasut wrote:
> >>> On Thursday, August 13, 2015 at 05:50:18 PM, vikasm wrote:
> >>>> Hi Marek,
> >>> 
> >>> Hi!
> >>> 
> >>>> On 08/12/2015 07:07 PM, Marek Vasut wrote:
> >>>>> On Thursday, July 16, 2015 at 04:27:29 AM, Vikas Manocha wrote:
> >>>>> 
> >>>>> Commit message is missing.
> >>>> 
> >>>> Actually subject of the mail was sufficient, this patch just moves the
> >>>> register configuration in init.
> >>> 
> >>> NAK, fix the commit message.
> >>> 
> >>>>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >>>>>> ---
> >>>>>> 
> >>>>>> Changes in v2: Rebased to master
> >>>>>> 
> >>>>>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
> >>>>>>  1 file changed, 2 insertions(+), 7 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/drivers/spi/cadence_qspi_apb.c
> >>>>>> b/drivers/spi/cadence_qspi_apb.c index d053407..1ae7edf 100644
> >>>>>> --- a/drivers/spi/cadence_qspi_apb.c
> >>>>>> +++ b/drivers/spi/cadence_qspi_apb.c
> >>>>>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> >>>>>> cadence_spi_platdata *plat)
> >>>>>> 
> >>>>>>  	/* Indirect mode configurations */
> >>>>>>  	writel((plat->sram_size/2), plat->regbase +
> >>>>>>  	CQSPI_REG_SRAMPARTITION);
> >>>>>> 
> >>>>>> +	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
> >>>>> 
> >>>>> You can drop the parenthesis around the first argument, they're
> >>>>> useless. Also, the indent of the second arg should be fixed, I
> >>>>> believe checkpatch might even complain about it.
> >> 
> >> ok for first comment about parenthesis but indent of second arg seems
> >> ok. yes, checkpatch warning was "CHECK: Alignment should match open
> >> parenthesis" but i ignored it. To respect 80 column, i had to move
> >> second arg in another line. Am i missing something ?
> > 
> > Just don't ignore the checkpatch warnings next time please ;-)
> 
> This CHECK message is gonna come in any case if i move second argument in
> second line to restrict 80 column rule. My understanding is to ignore this
> CHECK message.

I'll let others to present their opinion, I stop here.

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index d053407..1ae7edf 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -534,6 +534,8 @@  void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
 
 	/* Indirect mode configurations */
 	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
+	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
+				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Disable all interrupts */
 	writel(0, plat->regbase + CQSPI_REG_IRQMASK);
@@ -693,10 +695,6 @@  int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
 		/* for normal read (only ramtron as of now) */
 		addr_bytes = cmdlen - 1;
 
-	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
-
 	/* Configure the opcode */
 	rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
 
@@ -790,9 +788,6 @@  int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 		       cmdlen, (unsigned int)cmdbuf);
 		return -EINVAL;
 	}
-	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Configure the opcode */
 	reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB;