Re: [PATCH v5 1/6] drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux
On Thu, May 7, 2020 at 11:35 PM Douglas Anderson wrote: > The ti-sn65dsi86 MIPI DSI to eDP bridge chip has 4 pins on it that can > be used as GPIOs in a system. Each pin can be configured as input, > output, or a special function for the bridge chip. These are: > - GPIO1: SUSPEND Input > - GPIO2: DSIA VSYNC > - GPIO3: DSIA HSYNC or VSYNC > - GPIO4: PWM > > Let's expose these pins as GPIOs. A few notes: > - Access to ti-sn65dsi86 is via i2c so we set "can_sleep". > - These pins can't be configured for IRQ. > - There are no programmable pulls or other fancy features. > - Keeping the bridge chip powered might be expensive. The driver is > setup such that if all used GPIOs are only inputs we'll power the > bridge chip on just long enough to read the GPIO and then power it > off again. Setting a GPIO as output will keep the bridge powered. > - If someone releases a GPIO we'll implicitly switch it to an input so > we no longer need to keep the bridge powered for it. > > Because of all of the above limitations we just need to implement a > bare-bones GPIO driver. The device tree bindings already account for > this device being a GPIO controller so we only need the driver changes > for it. > > NOTE: Despite the fact that these pins are nominally muxable I don't > believe it makes sense to expose them through the pinctrl interface as > well as the GPIO interface. The special functions are things that the > bridge chip driver itself would care about and it can just configure > the pins as needed. > > Signed-off-by: Douglas Anderson > Cc: Linus Walleij > Cc: Bartosz Golaszewski Looks good mostly! > + pdata->gchip.label = dev_name(pdata->dev); > + pdata->gchip.parent = pdata->dev; > + pdata->gchip.owner = THIS_MODULE; > + pdata->gchip.of_xlate = tn_sn_bridge_of_xlate; > + pdata->gchip.of_gpio_n_cells = 2; > + pdata->gchip.free = ti_sn_bridge_gpio_free; > + pdata->gchip.get_direction = ti_sn_bridge_gpio_get_direction; > + pdata->gchip.direction_input = ti_sn_bridge_gpio_direction_input; > + pdata->gchip.direction_output = ti_sn_bridge_gpio_direction_output; > + pdata->gchip.get = ti_sn_bridge_gpio_get; > + pdata->gchip.set = ti_sn_bridge_gpio_set; > + pdata->gchip.can_sleep = true; > + pdata->gchip.names = ti_sn_bridge_gpio_names; > + pdata->gchip.ngpio = SN_NUM_GPIOS; Please add: pdata->gchip.base = -1; So it is clear that you use dynamically assigned GPIO numbers, with that: Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/6] drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux
On Thu 07 May 14:34 PDT 2020, Douglas Anderson wrote:
> The ti-sn65dsi86 MIPI DSI to eDP bridge chip has 4 pins on it that can
> be used as GPIOs in a system. Each pin can be configured as input,
> output, or a special function for the bridge chip. These are:
> - GPIO1: SUSPEND Input
> - GPIO2: DSIA VSYNC
> - GPIO3: DSIA HSYNC or VSYNC
> - GPIO4: PWM
>
> Let's expose these pins as GPIOs. A few notes:
> - Access to ti-sn65dsi86 is via i2c so we set "can_sleep".
> - These pins can't be configured for IRQ.
> - There are no programmable pulls or other fancy features.
> - Keeping the bridge chip powered might be expensive. The driver is
> setup such that if all used GPIOs are only inputs we'll power the
> bridge chip on just long enough to read the GPIO and then power it
> off again. Setting a GPIO as output will keep the bridge powered.
> - If someone releases a GPIO we'll implicitly switch it to an input so
> we no longer need to keep the bridge powered for it.
>
> Because of all of the above limitations we just need to implement a
> bare-bones GPIO driver. The device tree bindings already account for
> this device being a GPIO controller so we only need the driver changes
> for it.
>
> NOTE: Despite the fact that these pins are nominally muxable I don't
> believe it makes sense to expose them through the pinctrl interface as
> well as the GPIO interface. The special functions are things that the
> bridge chip driver itself would care about and it can just configure
> the pins as needed.
>
I'm working on a patch for supporting the PWM controller in the bridge
and as you say the muxing for that is better left internal to the bridge
driver.
> Signed-off-by: Douglas Anderson
> Cc: Linus Walleij
> Cc: Bartosz Golaszewski
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> Removed Stephen's review tag in v5 to confirm he's good with the way I
> implemented of_xlate.
>
> Changes in v5:
> - Use of_xlate so that numbers in dts start at 1, not 0.
>
> Changes in v4:
> - Don't include gpio.h
> - Use gpiochip_get_data() instead of container_of() to get data.
> - GPIOF_DIR_XXX => GPIO_LINE_DIRECTION_XXX
> - Use Linus W's favorite syntax to read a bit from a bitfield.
> - Define and use SN_GPIO_MUX_MASK.
> - Add a comment about why we use a bitmap for gchip_output.
>
> Changes in v3:
> - Becaue => Because
> - Add a kernel-doc to our pdata to clarify double-duty of gchip_output.
> - More comments about how powering off affects us (get_dir, dir_input).
> - Cleanup tail of ti_sn_setup_gpio_controller() to avoid one "return".
> - Use a bitmap rather than rolling my own.
>
> Changes in v2:
> - ("Export...GPIOs") is 1/2 of replacement for ("Allow...bridge GPIOs")
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 214 ++
> 1 file changed, 214 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 6ad688b320ae..4e8df948b3b8 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -4,9 +4,11 @@
> * datasheet: http://www.ti.com/lit/ds/symlink/sn65dsi86.pdf
> */
>
> +#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -54,6 +56,14 @@
> #define BPP_18_RGB BIT(0)
> #define SN_HPD_DISABLE_REG 0x5C
> #define HPD_DISABLE BIT(0)
> +#define SN_GPIO_IO_REG 0x5E
> +#define SN_GPIO_INPUT_SHIFT 4
> +#define SN_GPIO_OUTPUT_SHIFT0
> +#define SN_GPIO_CTRL_REG 0x5F
> +#define SN_GPIO_MUX_INPUT 0
> +#define SN_GPIO_MUX_OUTPUT 1
> +#define SN_GPIO_MUX_SPECIAL 2
> +#define SN_GPIO_MUX_MASK0x3
> #define SN_AUX_WDATA_REG(x) (0x64 + (x))
> #define SN_AUX_ADDR_19_16_REG0x74
> #define SN_AUX_ADDR_15_8_REG 0x75
> @@ -88,6 +98,35 @@
>
> #define SN_REGULATOR_SUPPLY_NUM 4
>
> +#define SN_NUM_GPIOS 4
> +#define SN_GPIO_PHYSICAL_OFFSET 1
> +
> +/**
> + * struct ti_sn_bridge - Platform data for ti-sn65dsi86 driver.
> + * @dev: Pointer to our device.
> + * @regmap: Regmap for accessing i2c.
> + * @aux: Our aux channel.
> + * @bridge: Our bridge.
> + * @connector:Our connector.
> + * @debugfs: Used for managing our debugfs.
> + * @host_node:Remote DSI node.
> + * @dsi: Our MIPI DSI source.
> + * @refclk: Our reference clock.
> + * @panel:Our panel.
> + * @enable_gpio: The GPIO we toggle to enable the bridge.
> + * @supplies: Data for bulk enabling/disabling our regulators.
> + * @dp_lanes: Count of dp_lanes we're using.
> + *
> + * @gchip:If we expose our GPIOs, this is used.
> + * @gchip_output: A cache of whether we've
Re: [PATCH v5 1/6] drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux
Quoting Douglas Anderson (2020-05-07 14:34:55) > The ti-sn65dsi86 MIPI DSI to eDP bridge chip has 4 pins on it that can > be used as GPIOs in a system. Each pin can be configured as input, > output, or a special function for the bridge chip. These are: > - GPIO1: SUSPEND Input > - GPIO2: DSIA VSYNC > - GPIO3: DSIA HSYNC or VSYNC > - GPIO4: PWM > > Let's expose these pins as GPIOs. A few notes: > - Access to ti-sn65dsi86 is via i2c so we set "can_sleep". > - These pins can't be configured for IRQ. > - There are no programmable pulls or other fancy features. > - Keeping the bridge chip powered might be expensive. The driver is > setup such that if all used GPIOs are only inputs we'll power the > bridge chip on just long enough to read the GPIO and then power it > off again. Setting a GPIO as output will keep the bridge powered. > - If someone releases a GPIO we'll implicitly switch it to an input so > we no longer need to keep the bridge powered for it. > > Because of all of the above limitations we just need to implement a > bare-bones GPIO driver. The device tree bindings already account for > this device being a GPIO controller so we only need the driver changes > for it. > > NOTE: Despite the fact that these pins are nominally muxable I don't > believe it makes sense to expose them through the pinctrl interface as > well as the GPIO interface. The special functions are things that the > bridge chip driver itself would care about and it can just configure > the pins as needed. > > Signed-off-by: Douglas Anderson > Cc: Linus Walleij > Cc: Bartosz Golaszewski > --- Reviewed-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
