Your patch contains a driver for the serial flash chip on the
wnr3500l, but it's kinda ugly (excessive ioremaps, duplicated defines,
global statics, ...).
If you could clean this up, it would have a chance of getting accepted.
Bernhard
2010/5/20 Tathagata Das <[email protected]>:
> What do you mean to say "clean up the flash driver you have" ?
>
> Regards,
> Tathagata
>
>
> Bernhard Loos wrote:
>>
>> 2010/5/17 Tathagata Das <[email protected]>:
>>
>>>
>>> Okay, so I will start working on those drivers (serial and wired). Does
>>> current version of OpenWRT trunk supports WNR3500L ? Otherwise I need to
>>> download latest trunk, modify it to run on WNR3500L and then concentrate
>>> on
>>> drivers.
>>>
>>
>> I doesn't but the difference from your version to latest trunk is pretty
>> small.
>>
>>
>>>
>>> If you elaborate the idea of spi master driver then it would help me to
>>> proceed faster.
>>>
>>
>> I checked out the flash controller code and it isn't plain spi, the
>> chipcommon core seems to have some control logic for the flash.
>> So if you clean up the flash driver you have, it should be enough.
>> Having a proper bus driver for the serial flash code would be nice,
>> but it's probably overkill.
>>
>>
>>>
>>> Regards,
>>> Tathagata
>>>
>>> Bernhard Loos wrote:
>>>
>>>>
>>>> 2010/5/17 Tathagata Das <[email protected]>:
>>>>
>>>>
>>>>>
>>>>> Hi,
>>>>> I have used source code of version 21251. It might be possible that
>>>>> some
>>>>> fixes has been made after that version which is not reflected in my
>>>>> code.
>>>>> So
>>>>> if I use latest code then that issue will be resolved.
>>>>>
>>>>>
>>>>
>>>> The bugs are in the patch, not in the code already in openwrt, so it
>>>> won't.
>>>>
>>>>
>>>>
>>>>>
>>>>> You said that kernel patch should not contain any code under #ifdef.
>>>>> But
>>>>> it
>>>>> may happen that those changes are specific to WNR3500L. Then what
>>>>> should
>>>>> be
>>>>> the porting process ?
>>>>>
>>>>>
>>>>
>>>> Something like this isn't wnr3500l speific:
>>>> +#ifndef CONFIG_WNR3500L
>>>> void plat_irq_dispatch(void)
>>>> +#else
>>>> +void asmlinkage plat_irq_dispatch(void)
>>>> +#endif
>>>>
>>>> If there is stuff specific to the wnr, you should wrap it in an
>>>> #ifdef-#endif, not #ifdef-#else-#endif, because it has to be possible
>>>> to build a kernel which supports both the old and socs.
>>>>
>>>>
>>>>
>>>>>
>>>>> If you tell how to move this patch into a proper form, I can do this.
>>>>> However I have also one WNR3500L, nothing else.
>>>>>
>>>>>
>>>>
>>>> Those are the worst problems, but you will spend a lot of time and
>>>> effort duplicating work I already did. I really suggest you spend your
>>>> time on some other area, as for example proper serial flash support (a
>>>> spi master driver) or network (both wired and wireless).
>>>>
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Tathagata
>>>>>
>>>>> Bernhard Loos wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> Hello
>>>>>> Thanks for you work, but as I already told you in the mail for your
>>>>>> last patch, this is not really useable as it is right now.
>>>>>> You can't simply go around and wrap everything you want to add in an
>>>>>> #ifdef, especially not things like that:
>>>>>> +#ifndef CONFIG_WNR3500L
>>>>>> void plat_irq_dispatch(void)
>>>>>> +#else
>>>>>> +void asmlinkage plat_irq_dispatch(void)
>>>>>> +#endif
>>>>>> This is ugly as hell and unneeded. Such a thing will never get
>>>>>> accepted into mainline.
>>>>>> Those ifdefs also make it impossible to build a kernel supporting both
>>>>>> the new and the old bus type. And this is even besides all the bugs
>>>>>> still present in this code.
>>>>>> I am working on getting this patch into a proper form, but it's a slow
>>>>>> process, partly because I don't have that much time and partially
>>>>>> because I only have an 4716 device and nothing older.
>>>>>> If you could turn you serial driver into a proper spi master driver
>>>>>> and hook the flash up to, this would be very helpful. Changing the
>>>>>> flash map shouldn't be necessary, the rootfs_data partition is created
>>>>>> automatically.
>>>>>>
>>>>>> Bernhard
>>>>>>
>>>>>>
>>>>>> 2010/5/14 Tathagata Das <[email protected]>:
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>> Attached is the kernel patch to support Netgear WNR3500L. All changes
>>>>>>> are
>>>>>>> made under CONFIG_WNR3500L.
>>>>>>> I have used trunk source code of version number 21251 to create this
>>>>>>> kernel
>>>>>>> patch.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Tathagata <[email protected]>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>>>
>>
>>
>
_______________________________________________
openwrt-devel mailing list
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
