On Mon, May 13, 2013 at 1:04 PM, Chao Xie <[email protected]> wrote: >>> + const struct of_device_id *of_id = >>> + of_match_device(pxa_pwm_of_match, &pdev->dev); >>> + unsigned int npwm; >>> + >>> + if (!of_id) >>> + return -ENODEV; >>> + >>> + npwm = (unsigned int)of_id->data; >>> + pwm->chip.npwm = (npwm & HAS_SECONDARY_PWM) ? 2 : 1; >>> + >>> + return 0; >>> +} >>> + >>> static int pwm_probe(struct platform_device *pdev) >>> { >>> const struct platform_device_id *id = platform_get_device_id(pdev); >>> @@ -144,7 +180,6 @@ static int pwm_probe(struct platform_device *pdev) >>> pwm->chip.dev = &pdev->dev; >>> pwm->chip.ops = &pxa_pwm_ops; >>> pwm->chip.base = -1; >>> - pwm->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1; >>> >>> r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (r == NULL) { >>> @@ -156,6 +191,20 @@ static int pwm_probe(struct platform_device *pdev) >>> if (IS_ERR(pwm->mmio_base)) >>> return PTR_ERR(pwm->mmio_base); >>> >>> + if (!id) { >>> + if (IS_ENABLED(CONFIG_OF)) >>> + ret = pxa_pwm_parse_data_dt(pdev, pwm); >>> + else >>> + ret = -ENODEV; >>> + } else { >>> + pwm->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 >>> : 1; >>> + } >> >> ^^ braces not necessarily here, and I'm not really sure if we should check >> CONFIG_OF firstly, and leave the platform_device_id thing as a legacy >> fall back, what do you think? >> >> if (IS_ENABLED(CONFIG_OF)) { >> ... >> } else { >> const struct platform_device_id *id = platform_get_device_id(...); >> ... >> } >> > it has some reasons. > You ways works for > 1. PWM defined in DT configuration file and CONFIG_OF is defined > 2. CONFIG_OF is not defined. > If COFNIG_OF is defined, but PWM is not defined in device tree > configuration file. The device > can still match the driver is the id_table is matched or name is matched. > So I covered addtional situation > 1. PWM is not defined in DT configuration file but CONFIG_OF is defined. > > So i keep the possiblility that event CONFIG_OF is defined, but PWM is > not defined in DT file, and we still can use old way to probe the > device. >
Yeah I was thinking maybe we should not keep the legacy working if CONFIG_OF is defined, but that should be OK: Acked-by: Eric Miao <[email protected]> >>> + >>> + if (ret) { >>> + dev_err(&pdev->dev, "failed to parse data from device >>> id\n"); >>> + return ret; >>> + } >>> + >>> ret = pwmchip_add(&pwm->chip); >>> if (ret < 0) { >>> dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); >>> @@ -181,6 +230,7 @@ static struct platform_driver pwm_driver = { >>> .driver = { >>> .name = "pxa25x-pwm", >>> .owner = THIS_MODULE, >>> + .of_match_table = pxa_pwm_of_match, >>> }, >>> .probe = pwm_probe, >>> .remove = pwm_remove, >>> -- >>> 1.7.4.1 >>> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
