On 07/16/2012 02:57 PM, Wolfgang Denk wrote:
> Dear Scott,
>
> In message <5000ab43.6090...@freescale.com> you wrote:
>>
>>> You are interpreting something which can be random data.
>>>
if (e.version == 0)
crc_offset = 0x72;
So here we're reading the 'version' field before w
Dear Scott,
In message <5000ab43.6090...@freescale.com> you wrote:
>
> > You are interpreting something which can be random data.
> >
> >> if (e.version == 0)
> >>crc_offset = 0x72;
> >>
> >> So here we're reading the 'version' field before we validate the data,
> >> because we need to check
Or Yoram-R56270 wrote:
> 1. Correct the v1 documentation to specify 23 MAC addresses and placing the
> CRC at 0xCC.
> 2. Create a new code and documentation for v2, specifying 31 MAC addresses
> and placing the CRC at 0xFC.
Both of these are unnecessary. My patch fixes the code to match the spe
On 07/13/2012 06:01 PM, Wolfgang Denk wrote:
> Dear Timur Tabi,
>
> In message <500098f6.8050...@freescale.com> you wrote:
>>
>> I honestly don't see what's wrong with checking the CRC in the old
>> location, and using it if it's valid. Like I said, we already
>
> You are interpreting something
Dear Timur Tabi,
In message <500098f6.8050...@freescale.com> you wrote:
>
> I honestly don't see what's wrong with checking the CRC in the old
> location, and using it if it's valid. Like I said, we already
You are interpreting something which can be random data.
> if (e.version == 0)
>
Dear Scott Wood,
In message <500096e0.9010...@freescale.com> you wrote:
>
> If not, and Wolfgang still refuses to accept this, what about checking
> the old location on a CRC fail, and if the old CRC passes, don't
> automatically use it but print a message telling the user that they
> probably nee
On 07/13/2012 05:46 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> But you continue to generate v1 EEPROMs. If we get the people who
>> define the format to accept v2, then we can generate v2 after the fix is
>> applied, and the (very small) risk of a real CRC failure combined with a
>> spurious CRC
Scott Wood wrote:
> But you continue to generate v1 EEPROMs. If we get the people who
> define the format to accept v2, then we can generate v2 after the fix is
> applied, and the (very small) risk of a real CRC failure combined with a
> spurious CRC success in the old location would only apply on
On 07/13/2012 05:22 PM, Timur Tabi wrote:
> Scott Wood wrote:
>
>> I know the spec wouldn't change, except the version number. But as I
>> said above, there would be no known v2 implementations with the bug.
>> You would only check the bad CRC location if you see v1 data, because
>> there are kno
Scott Wood wrote:
> I know the spec wouldn't change, except the version number. But as I
> said above, there would be no known v2 implementations with the bug.
> You would only check the bad CRC location if you see v1 data, because
> there are known buggy v1 implementations.
I already have that:
On 07/13/2012 04:53 PM, Timur Tabi wrote:
> Scott Wood wrote:
>
>> Timur, I know you said you don't control the format, but could you ask
>> for a version number bump so that going forward there's a way to
>> unambiguously mark the contents as "good" (the spec wouldn't change, but
>> there would b
Scott Wood wrote:
> Timur, I know you said you don't control the format, but could you ask
> for a version number bump so that going forward there's a way to
> unambiguously mark the contents as "good" (the spec wouldn't change, but
> there would be no known implementations of v2 with this bug)?
On 07/13/2012 04:25 PM, Wolfgang Denk wrote:
> Dear Tabi Timur-B04825,
>
> In message <50001038.50...@freescale.com> you wrote:
>>>
>> York is mistaken. The CRC was always at location 0xFC, but for some
>> reason, when I wrote the code, I put it at 0xCC. Now I'm fixing it, and
>> providing som
Wolfgang Denk wrote:
> Well, if it's really so unimportant and used in only a small number
> of boards, then just omit this broken code that provokes the
> undefined behaviour.
As I said before, we need to support situations where people upgrade their
U-Boot. When the EEPROM is read, the CRC is c
Dear Timur Tabi,
In message <50009349.9000...@freescale.com> you wrote:
>
> > In case you have an EEPROM with correct layout (CRC at 0xFC) but
> > incorrect CRC, you will access random data and interpret this as CRC.
> > This is provoking undefined behaviour.
>
> True, but it doesn't matter. Th
Wolfgang Denk wrote:
> In case you have an EEPROM with correct layout (CRC at 0xFC) but
> incorrect CRC, you will access random data and interpret this as CRC.
> This is provoking undefined behaviour.
True, but it doesn't matter. The EEPROM is not that important, and the
odds of screwing this up
Dear Tabi Timur-B04825,
In message <5000106b.5090...@freescale.com> you wrote:
>
> > It will work by chance, accessing random data. This is crap.
>
> It is not crap, and it will not work by chance. It is not accessing
> random data, it is accessing the CRC in the old location, just like the
>
Dear Tabi Timur-B04825,
In message <50001038.50...@freescale.com> you wrote:
> >
> York is mistaken. The CRC was always at location 0xFC, but for some
> reason, when I wrote the code, I put it at 0xCC. Now I'm fixing it, and
> providing some backwards compatibility to avoid causing problems fo
On Fri, 2012-07-13 at 05:12 -0700, Tabi Timur-B04825 wrote:
> sun york-R58495 wrote:
>
> > I agree it was a broken design. Now we are using all available space
> > and put CRC to the very end. It is not perfect but should work.
>
> *sigh*
>
> The design was never broken, the code was just wrong.
sun york-R58495 wrote:
> I agree it was a broken design. Now we are using all available space
> and put CRC to the very end. It is not perfect but should work.
*sigh*
The design was never broken, the code was just wrong. The CRC has always
supposed to have been at the end.
--
Timur Tabi
Linu
Wolfgang Denk wrote:
>> >My patch provides transparent updates to handle it. It will read broken
>> >EEPROMs and verify the CRC in the old location, and if you have re-save
>> >the EEPROM, it will put the CRC in the right place.
> It will work by chance, accessing random data. This is crap.
It
Wolfgang Denk wrote:
> This is a totally broken design then, when you have a growing data
> structure where vital information fields get shifted. In such case,
> the CRC should have been at the beginning, so it never changes
> location. Or even better, you should not have used a binary data
> stru
On Jul 12, 2012, at 9:30 PM, Wolfgang Denk wrote:
> Dear York,
>
> In message <9f5356fb-8ca2-44de-9089-64abd82ca...@freescale.com> you wrote:
>>
>> That patch itself is OK. But the comment is incorrect. We keep
>> adding more mac addresses to this data structure. The CRC was at the
>> end. The
Dear Timur Tabi,
In message <4fff5524.2080...@freescale.com> you wrote:
>
> My patch provides transparent updates to handle it. It will read broken
> EEPROMs and verify the CRC in the old location, and if you have re-save
> the EEPROM, it will put the CRC in the right place.
It will work by chan
Dear York,
In message <9f5356fb-8ca2-44de-9089-64abd82ca...@freescale.com> you wrote:
>
> That patch itself is OK. But the comment is incorrect. We keep
> adding more mac addresses to this data structure. The CRC was at the
> end. The offset 0xCC was correct.
This is a totally broken design then
On Jul 12, 2012, at 3:37 PM, Scott Wood wrote:
> On 07/12/2012 05:03 PM, sun york-R58495 wrote:
>> Timur,
>>
>> That patch itself is OK. But the comment is incorrect. We keep adding more
>> mac addresses to this data structure. The CRC was at the end. The offset
>> 0xCC was correct.
>
> Is th
Scott Wood wrote:
> If the 0xCC version is already in real use, then this change should bump
> the version number.
I can't, because I don't control the spec. Like I said, it was just wrong
before. No one has more than 23 MAC addresses anyway.
My patch provides transparent updates to handle it.
On 07/12/2012 05:46 PM, sun york-R58495 wrote:
>
> On Jul 12, 2012, at 3:37 PM, Scott Wood wrote:
>
>> On 07/12/2012 05:03 PM, sun york-R58495 wrote:
>>> Timur,
>>>
>>> That patch itself is OK. But the comment is incorrect. We keep adding more
>>> mac addresses to this data structure. The CRC wa
Scott Wood wrote:
>> > That patch itself is OK. But the comment is incorrect. We keep adding more
>> > mac addresses to this data structure. The CRC was at the end. The offset
>> > 0xCC was correct.
> Is there anything in the data structure to indicate that this growth has
> happened?
The versi
On 07/12/2012 05:03 PM, sun york-R58495 wrote:
> Timur,
>
> That patch itself is OK. But the comment is incorrect. We keep adding more
> mac addresses to this data structure. The CRC was at the end. The offset 0xCC
> was correct.
Is there anything in the data structure to indicate that this gro
Timur,
That patch itself is OK. But the comment is incorrect. We keep adding more mac
addresses to this data structure. The CRC was at the end. The offset 0xCC was
correct.
York
On Jul 12, 2012, at 2:46 PM, Timur Tabi wrote:
> The NXID v1 EEPROM format has the CRC at offset 0xFC, but for som
The NXID v1 EEPROM format has the CRC at offset 0xFC, but for some reason it
was placed at address 0xCC instead. To retain compatibility with existing
boards, we check the CRC in the old location if necessary.
Signed-off-by: Timur Tabi
---
board/freescale/common/sys_eeprom.c | 28
32 matches
Mail list logo
