[PATCH 1/3] pinctrl: add samsung pinctrl and gpiolib driver

On 1 June 2012 09:36, Linus Walleij [off-list ref] wrote:

On Thu, May 24, 2012 at 7:47 AM, Thomas Abraham
[off-list ref] wrote:

quoted

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt

It would be nice if some DT person could review the bindings, but if
nothing happens
I will accept them as-is.

This driver is special since it seems to keep all pin, group and
function information
in the device tree, but not like Tony is doing it in pinctrl-simple,
if I understand
correctly?

quoted

diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
new file mode 100644
index 0000000..480e2f0

(...)

quoted

+/* reading pin pull up/down and driver strength settings not implemented */
+static int samsung_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *config)
+{
+ ? ? ? return -ENOTSUPP;
+}
+
+/* reading pin pull up/down and driver strength settings not implemented */
+static int samsung_pinconf_group_get(struct pinctrl_dev *pctldev,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int group, unsigned long *config)
+{
+ ? ? ? return -ENOTSUPP;
+}

I think we made these optional now, so you can just leave them undefined
in the pinconf_ops.

Ok. I will leave them undefined.

quoted

+static inline struct samsung_pinctrl_drv_data *samsung_pinctrl_get_driver_data(
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct platform_device *pdev)
+{
+#ifdef CONFIG_OF
+ ? ? ? if (pdev->dev.of_node) {
+ ? ? ? ? ? ? ? int id;
+ ? ? ? ? ? ? ? const struct of_device_id *match;
+ ? ? ? ? ? ? ? const struct device_node *node = pdev->dev.of_node;
+
+ ? ? ? ? ? ? ? id = of_alias_get_id(pdev->dev.of_node, "pinctrl");
+ ? ? ? ? ? ? ? if (id < 0) {
+ ? ? ? ? ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to get alias id\n");
+ ? ? ? ? ? ? ? ? ? ? ? return NULL;
+ ? ? ? ? ? ? ? }
+ ? ? ? ? ? ? ? match = of_match_node(samsung_pinctrl_dt_match, node);
+ ? ? ? ? ? ? ? return (struct samsung_pinctrl_drv_data *)match->data + id;
+ ? ? ? }
+#endif

You don't need this #ifdef because in Kconfig the driver already depends on
OF so this is always available.

Yes, I missed that.

quoted

+/* driver data for various samsung soc's */
+#ifdef CONFIG_CPU_EXYNOS4210
+#define EXYNOS4210_PCTRL_DRVDATA (&exynos4210_pinctrl_drv_data)
+#else
+#define EXYNOS4210_PCTRL_DRVDATA (NULL)
+#endif /* CONFIG_CPU_EXYNOS4210 */

Can you do this in some .h file instead?

Ok. I will check on this.

quoted

+#ifdef CONFIG_OF

You don't need this because in Kconfig the driver depends on OF
already.

quoted

+static const struct of_device_id samsung_pinctrl_dt_match[] = {
+#ifdef CONFIG_CPU_EXYNOS4210
+ ? ? ? { .compatible = "samsung,pinctrl-exynos4210",
+ ? ? ? ? ? ? ? .data = (void *)exynos4210_pinctrl_drv_data },
+#endif /* CONFIG_CPU_EXYNOS4210 */
+ ? ? ? {},
+};
+MODULE_DEVICE_TABLE(of, samsung_pinctrl_dt_match);
+#endif /* CONFIG_OF */

So delete the closing #endif too.

Initially, it started with dt and non-dt support. Then switched over
to dt-only support and in the process, I left these artifacts. I will
clean this up.

Thank you Linus for your review and comments.

Regards,
Thomas.

Yours,
Linus Walleij