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 s
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