Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-26 Thread Dmitry Osipenko

26.01.2015 19:03, Wolfram Sang пишет:



Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3
without "val = 0", if all will be fine.


Please either send V3 or explicitly say V2 is OK. No need to hurry, just
saying that I am waiting for updates here...


Sure!

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-26 Thread Wolfram Sang

> Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3
> without "val = 0", if all will be fine.

Please either send V3 or explicitly say V2 is OK. No need to hurry, just
saying that I am waiting for updates here...



signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-26 Thread Dmitry Osipenko

26.01.2015 19:03, Wolfram Sang пишет:



Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3
without val = 0, if all will be fine.


Please either send V3 or explicitly say V2 is OK. No need to hurry, just
saying that I am waiting for updates here...


Sure!

--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-26 Thread Wolfram Sang

 Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3
 without val = 0, if all will be fine.

Please either send V3 or explicitly say V2 is OK. No need to hurry, just
saying that I am waiting for updates here...



signature.asc
Description: Digital signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Dmitry Osipenko

23.01.2015 16:27, Dmitry Osipenko пишет:

23.01.2015 12:45, Thierry Reding пишет:

On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:


Should this not technically be le32_to_cpu() since the data originates

>from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU
endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.


Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
be RAZ, however I don't see anything on it in documentation. In that case it
won't cause any problems with LE value and nullifying is only needed for BE
mode.


What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry


Got your point. I was thinking it's expected behavior, but now I'll elaborate
this more.

Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3 
without "val = 0", if all will be fine.


--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Dmitry Osipenko

23.01.2015 12:45, Thierry Reding пишет:

On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:


Should this not technically be le32_to_cpu() since the data originates

>from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.


Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
be RAZ, however I don't see anything on it in documentation. In that case it
won't cause any problems with LE value and nullifying is only needed for BE
mode.


What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry

Got your point. I was thinking it's expected behavior, but now I'll elaborate 
this more.


--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Thierry Reding
On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:
> 22.01.2015 19:06, Dmitry Osipenko пишет:
> >22.01.2015 18:22, Dmitry Osipenko пишет:
> >>22.01.2015 10:55, Alexandre Courbot пишет:
> >>>On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
> >>> wrote:
> 
> Should this not technically be le32_to_cpu() since the data originates
> from the I2C controller?
> >>
> >>No, i2c_readl returns value in CPU endianness, so it's correct. But for
> >>i2c_writel should be used le32_to_cpu(), since it takes value in CPU 
> >>endianness.
> >>It's my overlook, V2 is coming.
> >>
> 
> Why does this have to be initialized to 0 now?
> >>>
> >>>I suspect this is because we are going to memcpy less than 4 bytes
> >>>into it, but I cannot figure out how that memcpy if guaranteed to
> >>>produce the expected result for both endiannesses.
> >>>
> >>That's correct. Memcpy is working with bytes, so it doesn't care about
> >>endianness and produces expected result, since I2C message is char array.
> >>
> >I'll spend some more time reviewing, to see if nullifying should go as 
> >separate
> >patch.
> >
> Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
> be RAZ, however I don't see anything on it in documentation. In that case it
> won't cause any problems with LE value and nullifying is only needed for BE
> mode.

What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry


pgpk1o_K2E6Ws.pgp
Description: PGP signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Thierry Reding
On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:
 22.01.2015 19:06, Dmitry Osipenko пишет:
 22.01.2015 18:22, Dmitry Osipenko пишет:
 22.01.2015 10:55, Alexandre Courbot пишет:
 On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 thierry.red...@gmail.com wrote:
 
 Should this not technically be le32_to_cpu() since the data originates
 from the I2C controller?
 
 No, i2c_readl returns value in CPU endianness, so it's correct. But for
 i2c_writel should be used le32_to_cpu(), since it takes value in CPU 
 endianness.
 It's my overlook, V2 is coming.
 
 
 Why does this have to be initialized to 0 now?
 
 I suspect this is because we are going to memcpy less than 4 bytes
 into it, but I cannot figure out how that memcpy if guaranteed to
 produce the expected result for both endiannesses.
 
 That's correct. Memcpy is working with bytes, so it doesn't care about
 endianness and produces expected result, since I2C message is char array.
 
 I'll spend some more time reviewing, to see if nullifying should go as 
 separate
 patch.
 
 Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
 be RAZ, however I don't see anything on it in documentation. In that case it
 won't cause any problems with LE value and nullifying is only needed for BE
 mode.

What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry


pgpk1o_K2E6Ws.pgp
Description: PGP signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Dmitry Osipenko

23.01.2015 12:45, Thierry Reding пишет:

On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:


Should this not technically be le32_to_cpu() since the data originates

from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.


Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
be RAZ, however I don't see anything on it in documentation. In that case it
won't cause any problems with LE value and nullifying is only needed for BE
mode.


What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry

Got your point. I was thinking it's expected behavior, but now I'll elaborate 
this more.


--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-23 Thread Dmitry Osipenko

23.01.2015 16:27, Dmitry Osipenko пишет:

23.01.2015 12:45, Thierry Reding пишет:

On Thu, Jan 22, 2015 at 08:18:34PM +0300, Dmitry Osipenko wrote:

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:


Should this not technically be le32_to_cpu() since the data originates

from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU
endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.


Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to
be RAZ, however I don't see anything on it in documentation. In that case it
won't cause any problems with LE value and nullifying is only needed for BE
mode.


What does I2C_FIFO_STATUS have to do with anything?

My point was more that we already tell hardware how much data is to be
transferred (via the packet header in tegra_i2c_xfer_msg()), hence the
hardware shouldn't care whether the FIFO is padded with random data or
zeros.

Thierry


Got your point. I was thinking it's expected behavior, but now I'll elaborate
this more.

Gaahh! I'm sure it wasn't working before! I'll make more testing and send v3 
without val = 0, if all will be fine.


--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.

Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to be 
RAZ, however I don't see anything on it in documentation. In that case it won't 
cause any problems with LE value and nullifying is only needed for BE mode.


--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.

I'll spend some more time reviewing, to see if nullifying should go as separate 
patch.


--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for 
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.

It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.

That's correct. Memcpy is working with bytes, so it doesn't care about 
endianness and produces expected result, since I2C message is char array.


--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.

I'll spend some more time reviewing, to see if nullifying should go as separate 
patch.


--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for 
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.

It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.

That's correct. Memcpy is working with bytes, so it doesn't care about 
endianness and produces expected result, since I2C message is char array.


--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-22 Thread Dmitry Osipenko

22.01.2015 19:06, Dmitry Osipenko пишет:

22.01.2015 18:22, Dmitry Osipenko пишет:

22.01.2015 10:55, Alexandre Courbot пишет:

On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:


Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?


No, i2c_readl returns value in CPU endianness, so it's correct. But for
i2c_writel should be used le32_to_cpu(), since it takes value in CPU endianness.
It's my overlook, V2 is coming.



Why does this have to be initialized to 0 now?


I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.


That's correct. Memcpy is working with bytes, so it doesn't care about
endianness and produces expected result, since I2C message is char array.


I'll spend some more time reviewing, to see if nullifying should go as separate
patch.

Well, I2C_FIFO_STATUS returns 8-bit value. The rest of bits very likely to be 
RAZ, however I don't see anything on it in documentation. In that case it won't 
cause any problems with LE value and nullifying is only needed for BE mode.


--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-21 Thread Alexandre Courbot
On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
 wrote:
> On Tue, Jan 20, 2015 at 03:22:25PM +0300, Dmitry Osipenko wrote:
>> Support CPU BE mode by adding endianness conversion for memcpy interactions.
>>
>> Signed-off-by: Dmitry Osipenko 
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index 28b87e6..e0d3ef1 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -286,6 +286,7 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev 
>> *i2c_dev)
>>   if (rx_fifo_avail > 0 && buf_remaining > 0) {
>>   BUG_ON(buf_remaining > 3);
>>   val = i2c_readl(i2c_dev, I2C_RX_FIFO);
>> + val = cpu_to_le32(val);
>
> Should this not technically be le32_to_cpu() since the data originates
> from the I2C controller?
>
>>   memcpy(buf, , buf_remaining);
>>   buf_remaining = 0;
>>   rx_fifo_avail--;
>> @@ -343,7 +344,9 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev 
>> *i2c_dev)
>>*/
>>   if (tx_fifo_avail > 0 && buf_remaining > 0) {
>>   BUG_ON(buf_remaining > 3);
>> + val = 0;
>
> Why does this have to be initialized to 0 now?

I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-21 Thread Thierry Reding
On Tue, Jan 20, 2015 at 03:22:25PM +0300, Dmitry Osipenko wrote:
> Support CPU BE mode by adding endianness conversion for memcpy interactions.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/i2c/busses/i2c-tegra.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 28b87e6..e0d3ef1 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -286,6 +286,7 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev 
> *i2c_dev)
>   if (rx_fifo_avail > 0 && buf_remaining > 0) {
>   BUG_ON(buf_remaining > 3);
>   val = i2c_readl(i2c_dev, I2C_RX_FIFO);
> + val = cpu_to_le32(val);

Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?

>   memcpy(buf, , buf_remaining);
>   buf_remaining = 0;
>   rx_fifo_avail--;
> @@ -343,7 +344,9 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev 
> *i2c_dev)
>*/
>   if (tx_fifo_avail > 0 && buf_remaining > 0) {
>   BUG_ON(buf_remaining > 3);
> + val = 0;

Why does this have to be initialized to 0 now?

Thierry


pgpYy2UvwAmD5.pgp
Description: PGP signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-21 Thread Thierry Reding
On Tue, Jan 20, 2015 at 03:22:25PM +0300, Dmitry Osipenko wrote:
 Support CPU BE mode by adding endianness conversion for memcpy interactions.
 
 Signed-off-by: Dmitry Osipenko dig...@gmail.com
 ---
  drivers/i2c/busses/i2c-tegra.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
 index 28b87e6..e0d3ef1 100644
 --- a/drivers/i2c/busses/i2c-tegra.c
 +++ b/drivers/i2c/busses/i2c-tegra.c
 @@ -286,6 +286,7 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev 
 *i2c_dev)
   if (rx_fifo_avail  0  buf_remaining  0) {
   BUG_ON(buf_remaining  3);
   val = i2c_readl(i2c_dev, I2C_RX_FIFO);
 + val = cpu_to_le32(val);

Should this not technically be le32_to_cpu() since the data originates
from the I2C controller?

   memcpy(buf, val, buf_remaining);
   buf_remaining = 0;
   rx_fifo_avail--;
 @@ -343,7 +344,9 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev 
 *i2c_dev)
*/
   if (tx_fifo_avail  0  buf_remaining  0) {
   BUG_ON(buf_remaining  3);
 + val = 0;

Why does this have to be initialized to 0 now?

Thierry


pgpYy2UvwAmD5.pgp
Description: PGP signature


Re: [PATCH 1/2] i2c: tegra: Maintain CPU endianness

2015-01-21 Thread Alexandre Courbot
On Thu, Jan 22, 2015 at 4:40 PM, Thierry Reding
thierry.red...@gmail.com wrote:
 On Tue, Jan 20, 2015 at 03:22:25PM +0300, Dmitry Osipenko wrote:
 Support CPU BE mode by adding endianness conversion for memcpy interactions.

 Signed-off-by: Dmitry Osipenko dig...@gmail.com
 ---
  drivers/i2c/busses/i2c-tegra.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
 index 28b87e6..e0d3ef1 100644
 --- a/drivers/i2c/busses/i2c-tegra.c
 +++ b/drivers/i2c/busses/i2c-tegra.c
 @@ -286,6 +286,7 @@ static int tegra_i2c_empty_rx_fifo(struct tegra_i2c_dev 
 *i2c_dev)
   if (rx_fifo_avail  0  buf_remaining  0) {
   BUG_ON(buf_remaining  3);
   val = i2c_readl(i2c_dev, I2C_RX_FIFO);
 + val = cpu_to_le32(val);

 Should this not technically be le32_to_cpu() since the data originates
 from the I2C controller?

   memcpy(buf, val, buf_remaining);
   buf_remaining = 0;
   rx_fifo_avail--;
 @@ -343,7 +344,9 @@ static int tegra_i2c_fill_tx_fifo(struct tegra_i2c_dev 
 *i2c_dev)
*/
   if (tx_fifo_avail  0  buf_remaining  0) {
   BUG_ON(buf_remaining  3);
 + val = 0;

 Why does this have to be initialized to 0 now?

I suspect this is because we are going to memcpy less than 4 bytes
into it, but I cannot figure out how that memcpy if guaranteed to
produce the expected result for both endiannesses.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/