Hi Solomon,
One very minor nit.
On Sun, Jun 2, 2013 at 11:53 PM, Solomon Peachy <pizza@xxxxxxxxxxxx> wrote:
> The only advantage of 'struct resource' is that it lets us assign names
> as part of the platform data. Unfortunately since we are using platform
> data, we are already limited to a single instance of each driver,
> rendering this moot.
>
> So, replace the struct resources with ints, resulting in cleaner code.
>
> This was based on a suggestion from Arnd Bergmann.
>
> Signed-off-by: Solomon Peachy <pizza@xxxxxxxxxxxx>
> ---
> drivers/net/wireless/cw1200/cw1200_sagrad.c | 49 +++-----------------------
> drivers/net/wireless/cw1200/cw1200_sdio.c | 50 ++++++++++++---------------
> drivers/net/wireless/cw1200/cw1200_spi.c | 33 ++++++++----------
> include/linux/platform_data/cw1200_platform.h | 12 +++----
> 4 files changed, 47 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/net/wireless/cw1200/cw1200_sagrad.c b/drivers/net/wireless/cw1200/cw1200_sagrad.c
> index 14c2a18..3f884ac 100644
> --- a/drivers/net/wireless/cw1200/cw1200_sagrad.c
> +++ b/drivers/net/wireless/cw1200/cw1200_sagrad.c
> @@ -68,9 +45,9 @@ static struct cw1200_platform_data_sdio cw1200_platform_data = {
> .ref_clk = 38400,
> .have_5ghz = false,
> #if 0
> - .reset = &cw1200_href_resources[0],
> - .powerup = &cw1200_href_resources[1],
> - .irq = &cw1200_href_resources[2],
> + .reset = GPIO_RF_RESET, /* Replace as appropriate */
> + .powerup = GPIO_RF_POWERUP, /* Replace as appropriate */
> + .irq = GPIO_TO_IRQ(GPIO_RF_IRQ), /* Replace as appropriate */
You might want to mark these "Replace as appropriate" bits with a
comment at the start indicating that these need to be set for the
platform data or modified as appropriate. I haven't looked at the rest
of the driver, so you've probably documented this already, but it
can't hurt to say that twice.
Maybe:
/* If your device doesn't support Device Tree, you'll need to enable
* this and set the GPIO pins as appropriate.
*/
Also, at least in Vim, if "XXX", "FIXME" or "TODO" appear in comments,
they get highlighted, so it might be worth adding one of those,
(probably "TODO") to the comments so that this is a touch easier to
find.
Thanks,
--
Julian Calaby
Email: julian.calaby@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
