> -----Original Message----- > From: Sundar R IYER [mailto:sundar.iyer@xxxxxxxxxxxxxx] > Sent: Monday, September 06, 2010 8:46 PM > To: Datta, Shubhrajyoti > Cc: linux-input@xxxxxxxxxxxxxxx; STEricsson_nomadik_linux > Subject: RE: [PATCH v3 1/1] input: add support for Nomadik SKE keypad > controller > > Hi, > > >-----Original Message----- > >From: Datta, Shubhrajyoti [mailto:shubhrajyoti@xxxxxx] > >> + /* go through board initialiazation helpers */ > >> + if (keypad->board->init) { > >Did not understand this > > >-----Original Message----- > >From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] > >> > > + > >> > > + if (!keypad->board->init) { > >> > > + dev_err(&pdev->dev, "NULL board initialization > helper\n"); > >> > > >> > Could be checked earlier. Also, does it have to be an error? > >> > > >> > >> The platform code takes care to request the GPIOs and the GPIO > configurations. > >> Hence I return error in case the board configurations dont get > completed. > >> > > > >PLanform author migth opt to do the initialization earlier, together > >with teh rest of GPIO configuration so it is all ready by the time the > >driver is loaded. I think we should give such an option. > > I interpreted Dmitry's above suggestion as allowing the driver to proceed > in case platform > Initialization is not done. > > The [PATCH 2/2] ux500: add platform data for Nomadik SKE keypad controller > adds this platform > data which is configuration of the GPIO pins like this. > > +/* > + * ske_set_gpio_row: request and set gpio rows */ static int > +ske_set_gpio_row(int gpio) { > + int ret; > + > + ret = gpio_request(gpio, "ske-kp"); > + if (ret < 0) { > + pr_err("ske_set_gpio_row: gpio request failed\n"); > + return ret; > + } > + > + ret = gpio_direction_output(gpio, 1); > + if (ret < 0) { > + pr_err("ske_set_gpio_row: gpio direction failed\n"); > + gpio_free(gpio); > + } > + > + return ret; > +} > + > +/* > + * ske_kp_init - enable the gpio configuration */ static int > +ske_kp_init(void) { > + int ret, i; > + > + for (i = 0; i < SKE_KPD_MAX_ROWS; i++) { > + ret = ske_set_gpio_row(ske_kp_rows[i]); > + if (ret < 0) { > + pr_err("ske_kp_init: failed init\n"); > + return ret; > + } > + } > + > + return 0; > +} > > And hence I need to call this board init to make sure that I get the GPIO > lines > for my device. > > >> + > >> + if (keypad->board->exit()) > >> + keypad->board->exit(); > >What does this exit function do? > >Are you sure the exit function does not need the keypad clock? if (keypad->board->exit) keypad->board->exit(); Is this you intended? > > > > Although I haven't added a exit function to the platform data, but from > the platform > code posted from above, it is clear that there are no controller > dependencies for the > board exit function. > > Regards, > Sundar -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html