Hi Linus,
thanks for review
On 11/01/2017 15:55, Linus Walleij wrote:
On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> wrote:
+static void rz_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(chip->base + offset);
+
+ /* Set the GPIO as an input to ensure that the next GPIO request won't
+ * drive the GPIO pin as an output.
+ */
+ rz_gpio_direction_input(chip, offset);
+}
Very close to gpiochip_generic_free()
Maybe we should actually set lines as input in the
generic routine!
Is it ok then to simply substitute "pinctrl_free_gpio" with
"gpiochip_generic_free" and keep rz_gpio_direction_input here?
+ gpio_chip = &p->gpio_chip;
+ gpio_chip->direction_input = rz_gpio_direction_input;
+ gpio_chip->get = rz_gpio_get;
+ gpio_chip->direction_output = rz_gpio_direction_output;
Please implement .get_direction() as well.
That's funny how a simple addition like this one would require several
changes (possibly even in the dts ABI defined by this driver).
This is more a question for the original driver author I hope is
listening here:
To read the pin direction I would need to add one more register to the
"reg" property range provided in the DTS.
This is of course doable, but I would have two questions here:
- why did you chose to use PMSR register instead of reading/writing
directly to PM? Am I missing something?
- wouldn't it be better to just receive the port base address from the
"reg" property and offset from that instead of having the 3 register
addresses specified in the dts?
See, the dts now looks like this:
reg = <0xfcfe3100 0x4>, /* PSR */
<0xfcfe3200 0x2>, /* PPR */
<0xfcfe3800 0x4>; /* PMSR */
Shouldn't we just receive the gpiochip base address and the offset as we
like?
Thanks
j
