在 2022/11/24 6:14, Linus Walleij 写道:
On Wed, Nov 23, 2022 at 9:04 AM Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> wrote:
The Loongson platforms GPIO controller contains 60 GPIO pins in total,
4 of which are dedicated GPIO pins, and the remaining 56 are reused
with other functions. Each GPIO can set input/output and has the
interrupt capability.
This driver added support for Loongson GPIO controller and support to
use DTS or ACPI to descibe GPIO device resources.
Signed-off-by: Jianmin Lv <lvjianmin@xxxxxxxxxxx>
Signed-off-by: Hongchen Zhang <zhanghongchen@xxxxxxxxxxx>
Signed-off-by: Liu Peibao <liupeibao@xxxxxxxxxxx>
Signed-off-by: Juxin Gao <gaojuxin@xxxxxxxxxxx>
Signed-off-by: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx>
---
Change in v6:
This is way better :)
I guess you notice how the driver gets smaller and smaller.
This is a good sign!
It's thanks to your advice！
+static int loongson_gpio_request(
+ struct gpio_chip *chip, unsigned int pin)
+{
+ if (pin >= chip->ngpio)
+ return -EINVAL;
+
+ return 0;
+}
Drop this altogether as discussed in my other reply.
okay , I got it.
+static inline void __set_direction(struct loongson_gpio_chip *lgpio,
+ unsigned int pin, int input)
+static void __set_level(struct loongson_gpio_chip *lgpio, unsigned int pin,
+ int high)
I missed this before. Also the use of __underscore for inner functions
is a bad habit IMO (because __underscore is also used for compiler
primitives such as __init which is confusing) The signature of these
functions is too generic. Name them loongson_commit_direction() or
loongson_commit_level() or something.
okay , I got it.
+static int loongson_gpio_get_direction(
+ struct gpio_chip *chip, unsigned int pin)
thanks for implementing this!
+ if (lgpio->p_data->mode == BIT_CTRL_MODE) {
+ ret = bgpio_init(&lgpio->chip, dev, 8,
+ LOONGSON_GPIO_IN(lgpio),
+ LOONGSON_GPIO_OUT(lgpio), 0,
+ LOONGSON_GPIO_OEN(lgpio), NULL, 0);
+ if (ret) {
+ dev_err(dev, "unable to init generic GPIO\n");
+ return ret;
+ }
+ lgpio->chip.ngpio = ngpios;
Neat!
+ lgpio->chip.base = 0;
Drop this. It is good that the base is unpredictable so
people don't start to rely on it. (drivers/gpio/TODO)
okay, I got it.
+ rval = device_property_read_u16_array(dev, "gsi_idx_map", NULL, 0);
But this gsi_idx_map is missing from your device tree bindings,
is it not?
Or what am I missing here? Sorry I might overlook something...
gsi_idx_map is for acpi, the dts doesn't use it, and acpi should be
follow dts,
so remove it.
+static int loongson_gpio_probe(struct platform_device *pdev)
+{
+ void __iomem *reg_base;
+ struct loongson_gpio_chip *lgpio;
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+
+ lgpio = devm_kzalloc(dev, sizeof(*lgpio), GFP_KERNEL);
+ if (!lgpio)
+ return -ENOMEM;
+
+ loongson_gpio_get_props(pdev, lgpio);
+
+ lgpio->p_data = device_get_match_data(&pdev->dev);
lgpio->p_data = device_get_match_data(dev);
+static int __init loongson_gpio_setup(void)
+{
+ return platform_driver_register(&loongson_gpio_driver);
+}
+postcore_initcall(loongson_gpio_setup);
Why does this have to be postcore_initcall()?
it was refer other gpio driver. other perpherial driver will use gpio,
the gpio driver
should be loaded as early as possible,so use postcore_initcall();
Yours,
Linus Walleij
