On Thu, Jul 7, 2011 at 17:54, Jonathan Stroud wrote:
> On Jul 7, 2011, at 4:28 PM, Mike Frysinger wrote:
>> On Thu, Jul 7, 2011 at 16:47, Jonathan Stroud wrote:
>>> +#define ASHIFT ((bus_params_t *) bus->params)->ashift
>>> +#define DSHIFT ((bus_params_t *) bus->params)->dshift
>>
>> i'm not grasping why the shift info needs to be in the SPI bus layer.
>> could you elaborate on that ?
>
> Defines the number off address and data bytes that the device expects. You
> can
> set this when you initialize the bus. This is similar to the prototype bus
> where you
> tell it the size of the address bus and data buses.
the jedec spec (which all the spi flashes ive worked with support)
supports 8bit transfers to get the idcode out, and from there you know
exactly what manufacturer it is and its device id. and all the spi
flashes ive used work in 8bit mode. this is also how the u-boot spi
flash driver operates, as well as the linux kernel spi flash driver.
then for addresses, the first three bytes after the command are simply
the addresses:
static void spi_flash_addr(u32 addr, u8 *cmd)
{
/* cmd[0] is actual command */
cmd[1] = addr >> 16;
cmd[2] = addr >> 8;
cmd[3] = addr >> 0;
}
keeping the overrides on these is fine, but the default should match
this behavior (maybe it does already and i just didnt read the code
close enough?).
>>> + case 0: // "auto"
>>> + ASHIFT = 1;
>>> + break;
>>
>> we should be able to figure out the address shift based on the flash
>> type. although every one
>
> Not all devices provide this information. Easier just to tell the bus
> driver how to operate. This
> matches the behavior of the prototype bus driver.
true, but since we're going to have to have a structure of known spi
flashes that match the jedec id to the geometry, we can keep that info
in there too. so the default (automatic) behavior will pull the info
from the spi flash driver.
so we'll have to have either the bus pull info from the flash when
ASHIFT is 0, or have the spi flash driver push it into the bus. we
need to sort this out anyways for the cfi flashes as hardcoding
8/16/32 bit widths into specific drivers is silly.
> Frick... I put that in. Then missed it when I merged my code back to
> mainline.
> Adding it back to the next patch.
> I have a local branch with many more mods. This is why I have made some
> mistakes merging. Trying to be careful.
if you use git, you should be able to keep things sorted out much
easier. svn is certainly a pain for managing more than one patch
locally at a time, but git allows you to rebase your current work onto
the latest mainline and keep patches separate.
>>> + asize = (uint64_t)1<<asize;
>>
>> spaces around the "<<"
>
> nothing gets past you. its hard to code in someone else's style when you
> code in another all the time.
it's not my style ;), it's just what the urjtag code base adopted
before me. i prefer linux kernel style myself ...
>>> + spimem_write_byte (bus, (uint8_t)(data));
>>
>> no need for the uint8_t cast
>
> Its a uint32_t getting passed to a uint8_t. I would prefer to have it, but
> if you really want it gone, fine.
right, but C says the result is the same. passing a uint32_t to a
uint8_t implicitly casts to uint8_t. i prefer to avoid unnecessary
casts as my personal experience has shown they cause more harm than
good (people add casts to "fix" warnings, and then the surrounding
code changes, but the forced cast keeps the new warnings from showing
up).
-mike
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development
