[U-Boot] [PATCH v2 7/8] FAT: Simplify get_contents

[Tom Rini <trini@ti.com>]

On 09/04/2012 03:53 PM, Beno?t Th?baudeau wrote:
> On Wednesday, September 5, 2012 12:10:41 AM, Tom Rini wrote:
>> On 09/04/2012 03:07 PM, Beno?t Th?baudeau wrote:
>>> On Tuesday, September 4, 2012 10:50:34 PM, Tom Rini wrote:
>>>> On Sun, Sep 02, 2012 at 05:25:20PM +0200, Wolfgang Denk wrote:
>>>>> Dear Beno??t Th??baudeau,
>>>>>
>>>>> In message
>>>>> <1663419836.332713.1342790497668.JavaMail.root@advansee.com> you
>>>>> wrote:
>>>>>> One call to get_cluster can be factorized with another, so avoid
>>>>>> duplicatin> g
>>>>>> code.
>>>>>>
>>>>>> Signed-off-by: Beno??t Th??baudeau
>>>>>> <benoit.thebaudeau@advansee.com>
>>>>>> Cc: Wolfgang Denk <wd@denx.de>
>>>>>> ---
>>>>>> Changes for v2:
>>>>>>  - Patch renumbering because of the new v2 1/8.
>>>>>>  - Possible code style changes due to the new v2 1/8.
>>>>>>
>>>>>>  .../fs/fat/fat.c                                   |   14
>>>>>>  +-------------
>>>>>>  1 file changed, 1 insertion(+), 13 deletions(-)
>>>>>
>>>>> Applied, thanks.
>>>>
>>>> OK, this change is NOT equivalent code.  My platforms now hang
>>>> thusly
>>>> (with DEBUG set):
>>>> reading u-boot.img
>>>> VFAT Support enabled
>>>> FAT16, fat_sect: 4, fatlength: 144
>>>> Rootdir begins at cluster: 0, sector: 292, offset: 24800
>>>> Data begins at: 316
>>>> Sector size: 512, cluster size: 4
>>>> FAT read sect=292, clust_size=4, DIRENTSPERBLOCK=16
>>>> Rootvfatname: |u-boot.ais|
>>>> RootMismatch: |u-boot.ais|u-boot.ais|
>>>> RootMismatch: |u-boot.ais||
>>>> RootMismatch: |mlo||
>>>> Rootvfatname: |u-boot.img|
>>>> RootName: u-boot.img, start: 0xc2, size:  0x337d0
>>>> Filesize: 210896 bytes
>>>> 64 bytes
>>>> gc - clustnum: 194, startsect: 1092
>>>> Size: 210896, got: 64
>>>>
>>>> This is all fine in full U-Boot.
>>>
>>> OK. I'm looking into it.
>>>
>>> Can you give more details, like the type of storage (usb, mmc,
>>> etc.)? Do you
>>> have a command line and a disk image that could be used to
>>> duplicate the issue?
>>
>> It's an SD card.  If you have any "OMAP" platform (beagleboard,
>> beaglebone, pandaboard) or am35x/am37x or similar platforms SPL
>> should
>> hang like that.
> 
> Unfortunately, I don't have these platforms.
> 
>>  72MB partition (or so) on either a 2 or 4GB card.
>> Getting all the way up into U-Boot clears the problem away until
>> power
>> cycle.  That last part makes me worried...
> 
> What do you mean by "Getting all the way up into U-Boot"? Is it that your SPL
> can be aborted and give a command prompt?
> 
> The only difference that this specific patch (7/8) introduces in your use case
> (i.e. a request to read the first 64 bytes of a file of 210896 bytes) is that it
> removes a spurious call to get_cluster() with a size of 0, which should be
> transparent.
> 
> Can you confirm that your tests bisect to this patch and not to another one in
> this series?
> 
> Can you try to apply locally 5/8 and 8/8 to see if it makes a difference, just
> in case there would be some dependencies?
> 
> Is it OK to send e-mails with attachments on the ML? If so, I can post the full
> fat.c that I currently use so that you can test with it. I have used it for a
> couple of years without any issue.
> 
> Also, why do you read only 64 bytes from this file? According to your debug
> trace, this read was successful, and then no error is shown. So, if it hangs, it
> might be that the read data is corrupted, or that the image is not fully read
> because of this 64-byte limit.

I've bisected this twice and I've come to a conclusion.  It is this
patch that makes my problem show up, but it's not this patch at fault.
What I'm going to go with is that the "am33xx: Remove redundant timer
config" patch that I forgot in my last round of pull requests needs to
come in as with that set of patches applied locally to top of tree the
platform is fine again.

-- 
Tom