On Fri, Jan 11, 2019 at 09:48:21AM +0100, Johan Hovold wrote:
On Thu, Jan 10, 2019 at 11:23:07PM +0530, Nishad Kamdar wrote:
Use the gpiod interface instead of the deprecated old non-descriptor interface while continuing to ignore gpio flags from device tree in "svc_reset_onoff()" for now.
Signed-off-by: Nishad Kamdar nishadkamdar@gmail.com
Changes in v5:
- Change the commit message.
- Restore the names of the gpio device-tree properties without the "-gpio" suffix.
Changes in v4:
- Move 'gpio_desc *svc_sysboot' below the reset flag as it is more logical to have reset flag below reset gpio.
- Remove a few unnecessary line breaks.
Changes in v3:
- Add this patch to a patchset.
Changes in v2:
- Move comment to the same line as to what it applies to.
Also looks good. Some really minor comments to a couple of the error messages. The issues are there in the current code, but since you modify these messages anyway you might as well fix them up. Please fix that and resend with my
Reviewed-by: Johan Hovold johan@kernel.org
Ok, I'll do that.
Really good job with this!
Thank you.
@@ -435,6 +428,7 @@ static int arche_platform_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; int ret;
- unsigned int flags;
arche_pdata = devm_kzalloc(&pdev->dev, sizeof(*arche_pdata), GFP_KERNEL); @@ -444,61 +438,33 @@ static int arche_platform_probe(struct platform_device *pdev) /* setup svc reset gpio */ arche_pdata->is_reset_act_hi = of_property_read_bool(np, "svc,reset-active-high");
- arche_pdata->svc_reset_gpio = of_get_named_gpio(np,
"svc,reset-gpio",0);- if (!gpio_is_valid(arche_pdata->svc_reset_gpio)) {
dev_err(dev, "failed to get reset-gpio\n");return arche_pdata->svc_reset_gpio;- }
- ret = devm_gpio_request(dev, arche_pdata->svc_reset_gpio, "svc-reset");
- if (ret) {
dev_err(dev, "failed to request svc-reset gpio:%d\n", ret);return ret;- }
- ret = gpio_direction_output(arche_pdata->svc_reset_gpio,
arche_pdata->is_reset_act_hi);- if (ret) {
dev_err(dev, "failed to set svc-reset gpio dir:%d\n", ret);
- if (arche_pdata->is_reset_act_hi)
flags = GPIOD_OUT_HIGH;- else
flags = GPIOD_OUT_LOW;- arche_pdata->svc_reset = devm_gpiod_get(dev, "svc,reset", flags);
- if (IS_ERR(arche_pdata->svc_reset)) {
ret = PTR_ERR(arche_pdata->svc_reset);dev_err(dev, "failed to request svc-reset GPIO:%d\n", ret);Add a space after ':' for consistency.
Ok. I'll do that.
@@ -515,19 +481,11 @@ static int arche_platform_probe(struct platform_device *pdev) arche_pdata->num_apbs = of_get_child_count(np); dev_dbg(dev, "Number of APB's available - %d\n", arche_pdata->num_apbs);
- arche_pdata->wake_detect_gpio = of_get_named_gpio(np,
"svc,wake-detect-gpio",0);- if (arche_pdata->wake_detect_gpio < 0) {
dev_err(dev, "failed to get wake detect gpio\n");return arche_pdata->wake_detect_gpio;- }
- ret = devm_gpio_request(dev, arche_pdata->wake_detect_gpio,
"wake detect");- if (ret) {
dev_err(dev, "Failed requesting wake_detect gpio %d\n",arche_pdata->wake_detect_gpio);
- arche_pdata->wake_detect = devm_gpiod_get(dev, "svc,wake-detect",
GPIOD_IN);- if (IS_ERR(arche_pdata->wake_detect)) {
ret = PTR_ERR(arche_pdata->wake_detect);dev_err(dev, "Failed requesting wake_detect GPIO %d\n", ret);Add colon after "GPIO" for consistency (and to avoid ambiguity).
Ok, I'll do that.
Thanks for the review, Nishad
return ret;}
Thanks, Johan