<
<Overall this driver seems to be based on the IP vendor driver? It
<looks like a completely flexible driver that implements all possible
<combinations of everything.
<
[Tirumala Marri] Some what true that it was based on skeletal driver
Provided from IP vendor.
<And as a result, it's huge, and it's got a lot of extra code in there
<that I'm
<willing to bet that you have never even executed on your platform.
<
<Please, pare it down to the portions that you have used, and know works
<and can support. If others need the extra functionality in the future,
<they can and will expand and bring in what is needed.
<
[Tirumala Marri] I can sure review and find if there are any dead
functions.
It may not be 100% free of dead code as some of the code paths may execute
Asynchronously.
<Compare this to the dwc3 driver, which is much much cleaner.
<
[Tirumala Marri] I will check.
<Overall other comments:
<
<* Register definitions are crazy long. It means you have to do lots of
<line
<  wraps to keep the 80-character limit, which makes it hard to read the
<code.
[Tirumala Marri] This was suggestion from the review to use bit shifting.
I welcome any suggestions.
<* The header files seem to have been autogenerated and have unnneeded
<  shift/mask operations.
<
<* It doesn't build on non-powerpc platforms since it uses out_{b,l}e
<accessors.
<
[Tirumala Marri] You have to select Little Endian mode for LE platform
>From make menuconfig.

Thx,
Marri
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to