On 09/10/2012 11:52, Lukasz Majewski wrote:
> Hi Stefano,
>
>> On 05/10/2012 10:16, Lukasz Majewski wrote:
>>> Since the pmic_reg_read is the u32 value, the order in which bytes
>>> are placed to form u32 value is important.
>>>
>>> This commit adds the reverse (which is default) and normal byte
>>> order.
>>>
>>> Signed-off-by: Lukasz Majewski <l.majew...@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
>>>
>>> ---
>>
>> Hi Lucasz,
>>
>>> Changes for v2:
>>> - Move byte_order variable to struct pmic
>>
>> Is the byte reversal you introduce here only another way for the
>> endianess ?
>>
>> It seems to me you have a PMIC whose endianess is different as the
>> SOC's endianess, and that must be changed.
>>
>> I can suppose that "normal byte" and "reverse byte" works only with
>> ARM SOC, as they are (normally, but not always) little endian, but
>> not for example on PowerPC (big endian).
>
> This procedure was necessary since, some sensor produces output of 2B
> and those bytes are in a big endian.
Right. So we have the endianess of the SOC and the endianess of the
PMIC, and they can differ.
> For simplicity reason I decided to make the switch on received bytes.
>
> I suppose that witch some luck :-), proper cpu_to_le16|be16 (le32|be32)
> functions can be used to avoid repetition.
I hope so.
>
> To make the code working at both ARM and PPC we shall use the
> __BYTE_ORDER == __LITTLE_ENDIAN check, which is CPU dependent. Hence,
> the interpretation of stored data would be consistent from CPU point of
> view.
>
> To sum up - two options possible:
> 1. Use cpu_to_le|be functions (hack cpu_to_le32 to handle 3B input)
> 2. Perform switch (as it was done in this patch) with explicit check of
> __BYTE_ORDER == __LITTLE_ENDIAN.
>
> I would opt for 1 here.
I vote for 1, too. I think generally in u-boot and kernel there is no
use of the explicit check, but I have not grepped in code.
>
>>
>> If this is correct, then I think we need a parameter in the structure
>> to indicate the endianess,
>
> We need also the endiness of data coming out from sensor as it is
> defined in this patch:
> enum { PMIC_BYTE_ORDER_REVERSED, PMIC_BYTE_ORDER_NORMAL, };
> I admit, that name could be more appropriate here (like
> SENSOR_BYTE_ORDER).
Yes. The name is misleading. Which is "normal" nowadays ?
>
> In this patch I've added a "byte_order" variable to struct pmic to
> indicate the endiantess of device (I2C connected) to which we are
> talking to. It is per sensor defined (as the struct pmic is)
I think the procedure is correct. The endianess is stored in the pmic
structure, and then calling appropriate cpu_to_le/be function we should
have the bytes in the right order.
>
> Please feel free to comment other patches, so I could prepare v3 of
> this series :-).
I will do..
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
