"Hugo Villeneuve" <[EMAIL PROTECTED]> writes:
> Kevin Hilman wrote:
>> [EMAIL PROTECTED] writes:
>>
>>> From: Hugo Villeneuve <[EMAIL PROTECTED]>
>>>
>>> ARM DaVinci: Add support for the Lyrtech SFFSDR board
>>>
>>> The Lyrtech SFFSDR (Small form Factor Software Defined Radio) board
>>> is built around a TI DM6446 and a Xilinx Virtex-4 FPGA
>>>
>>> Signed-off-by: Hugo Villeneuve <[EMAIL PROTECTED]>
>>
>> Hi Hugo,
>>
>> Thanks for the support for this new board. I'm just about ready to
>> push your board support, but I'd like a few minor changes.
>>
>> Your patch does some of the same things that the dm355 patch does that
>> I will be reworking slightly.
>>
>> Could you please rework your patch on top of the 'tmp/ti-staging-2'
>> branch? In doing so, you can rework some things that I will note
>> below.
>
> Ok, no problem. I will do that today or during the weekend.
>
>>> +/* other misc. init functions */
>>> +void __init davinci_psc_init(void);
>>> +void __init davinci_irq_init(void);
>>> +void __init davinci_map_common_io(void);
>>> +void __init davinci_init_common_hw(void);
>>
>> Please drop these declarations. Instead, add
>>
>> #include <mach/serial.h>
>> #include <mach/psc.h>
>>
>> Which contain the declarations.
>
> Ok.
>
>>> +#if defined(CONFIG_MTD_NAND_DAVINCI)
>>
>> Your #if only catches when NAND is built-in, so it wont work if NAND
>> is built as a module.
>>
>> But in any case, you should drop these #ifs all together in board init
>> code. The upstream preference is to always compile these in just in
>> case a driver is built later and then can be loaded as a module.
>
> Ok, it makes sense.
>
>>> +#if defined(CONFIG_TI_DAVINCI_EMAC)
>>
>> Please drop the #ifdef
>
> Ok.
>
>>> +/* Get Ethernet address from kernel boot params */
>>> +static u8 davinci_sffsdr_mac_addr[6] = {0xFF, 0xFF, 0xFF, 0xFF,
>>> 0xFF, 0xFF}; +#endif /* CONFIG_TI_DAVINCI_EMAC) */
>>> +
>>> +static struct at24_platform_data eeprom_info = {
>>> + .byte_len = (64*1024) / 8,
>>> + .page_size = 32,
>>> + .flags = AT24_FLAG_ADDR16,
>>> +};
>>> +
>>> +static struct i2c_board_info __initdata i2c_info[] = { + {
>>> + I2C_BOARD_INFO("24LC64", 0x50),
>>> + .platform_data = &eeprom_info,
>>> + },
>>> + /* Other I2C devices:
>>> + * MSP430, addr 0x23
>>> + * PCA9543, addr 0x70 (setup done by U-Boot)
>>> + * ADS7828, addr 0x48 (ADC for voltage monitoring.) + */
>>> +};
>>
>> Does your board also have a MAC in this EEPROM? If so could
>> you use that like the dm6446 EVM board code does?
>
> The thing is that we must parse a structure and do some computations
> to extract the MAC address, and this code is already present in
> U-Boot. I think it is more logical for the bootloader to simply pass
> the MAC address to the kernel, instead of duplicating the code that
> extracts the MAC address from the EEPROM.
OK, that makes sense. Thanks for the clarification.
> Is there an official mechanism that we could use to pass information
> like that from U-Boot to Linux?
Not that I'm aware of. Kernel command-line eth= is OK with me.
Kevin
>>> +/* The msp430 uses a slow bitbanged I2C implementation (ergo 20
>>> KHz), + * which requires 100 usec of idle bus after i2c writes sent
>>> to it. + */
>>
>> Does your board really have the same msp430 firmware problem as the
>> DM6446 EVM? Or is this a cut-and-paste problem?
>
> Our board also has a MSP430 with a bitbang I2C, but it currently doesn´t
> work. I will remove that part.
>
>>> +static struct davinci_i2c_platform_data i2c_pdata = { + .bus_freq
>>> =
>>> 20 /* kHz */, + .bus_delay = 100 /* usec */,
>>> +};
>>> +
>>> +static void __init sffsdr_init_i2c(void)
>>> +{
>>> + davinci_init_i2c(&i2c_pdata);
>>> + i2c_register_board_info(1, i2c_info, ARRAY_SIZE(i2c_info)); +}
>>> +
>>> +static struct platform_device *davinci_sffsdr_devices[] __initdata
>>> = { +#if defined(CONFIG_MTD_NAND_DAVINCI)
>>
>> Drop #if
>>> + &davinci_sffsdr_nandflash_device,
>>> +#endif
>>> +};
>>> +
>>> +static struct davinci_uart_config davinci_sffsdr_uart_config
>>> __initdata = { + .enabled_uarts = (1 << 0), +};
>>> +
>>> +static struct davinci_board_config_kernel davinci_sffsdr_config[]
>>> __initdata = { + { DAVINCI_TAG_UART, &davinci_sffsdr_uart_config },
>>> +}; +
>>> +static void __init
>>> +davinci_sffsdr_map_io(void)
>>> +{
>>> + davinci_map_common_io();
>>> +}
>>> +
>>> +static __init void davinci_sffsdr_init(void)
>>> +{
>>> + davinci_psc_init();
>>> + platform_add_devices(davinci_sffsdr_devices,
>>> + ARRAY_SIZE(davinci_sffsdr_devices));
>>> + sffsdr_init_i2c();
>>> + davinci_board_config = davinci_sffsdr_config;
>>> + davinci_board_config_size = ARRAY_SIZE(davinci_sffsdr_config);
>>> + davinci_serial_init(); +
>>> +#if defined(CONFIG_TI_DAVINCI_EMAC)
>>> + davinci_init_emac(davinci_sffsdr_mac_addr);
>>> +#endif /* CONFIG_TI_DAVINCI_EMAC) */
>>
>> Drop #if
>>
>>> +#if defined(CONFIG_USB_MUSB_HDRC)
>>> + /* The SFFSDR supports only peripheral mode. */
>>> + setup_usb(0, 0);
>>> +#endif /* CONFIG_USB_SUPPORT */
>>
>> Drop #if
>>
>>> +}
>>> +
>>> +#if defined(CONFIG_TI_DAVINCI_EMAC)
>>
>> Drop #if
>>
>>> +/* Conversion of a MAC address from a string (AA:BB:CC:DD:EE:FF) +
>>> * to a 6 bytes array. */ +int davinci_sffsdr_convert_macaddr(char
>>> *mac_addr_str, u8 *mac_addr_array) +{ + int i;
>>> +
>>> + if (mac_addr_str == NULL)
>>> + return -EFAULT;
>>> +
>>> + for (i = 0; i < 6; i++)
>>> + mac_addr_array[i] = simple_strtol(&mac_addr_str[i*3], +
>>>
>>> (char **)NULL, 16); +
>>> + return 0;
>>> +}
>>> +
>>> +static int davinci_cpmac_eth_setup(char *str)
>>> +{
>>> + /* The first char passed from the bootloader is '=', so ignore it
>>> */ + davinci_sffsdr_convert_macaddr(&str[1],
>>> davinci_sffsdr_mac_addr); + + return 1;
>>> +}
>>> +
>>> +__setup("eth", davinci_cpmac_eth_setup);
>>> +#endif /* CONFIG_TI_DAVINCI_EMAC) */
>>> +
>>> +static __init void davinci_sffsdr_irq_init(void)
>>> +{
>>> + davinci_init_common_hw();
>>> + davinci_irq_init();
>>> +}
>>> +
>>> +MACHINE_START(SFFSDR, "Lyrtech SFFSDR")
>>> + /* Maintainer: Hugo Villeneuve [EMAIL PROTECTED] */
>>> + .phys_io = IO_PHYS, + .io_pg_offst = (__IO_ADDRESS(IO_PHYS)
>>> >> 18) & 0xfffc, + .boot_params = (DAVINCI_DDR_BASE + 0x100),
>>> + .map_io = davinci_sffsdr_map_io,
>>> + .init_irq = davinci_sffsdr_irq_init,
>>> + .timer = &davinci_timer,
>>> + .init_machine = davinci_sffsdr_init,
>>> +MACHINE_END
>>>
>>> _______________________________________________
>>> Davinci-linux-open-source mailing list
>>> Davinci-linux-open-source@linux.davincidsp.com
>>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
> Thanks for your comments.
>
> Hugo V.
>
> Hugo Villeneuve
> Hardware developer | Concepteur matériel
> Lyrtech
> Phone/Tél. : (1) (418) 877-4644 #2395
> Toll-free/Sans frais - Canada & USA : (1) (888) 922-4644 #2395
> Fax/Téléc. : (1) (418) 877-7710
> www.lyrtech.com
> Infinite possibilities...TM
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
