Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Noralf TrÃnnes]

Den 02.08.2018 21.45, skrev Sam Ravnborg:
> Add driver for the winstar wg160160 display.
> The driver utilises pardata-dbi that
> again utilise the pardata subsystem.
>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> ---
>   MAINTAINERS                        |   5 +
>   drivers/gpu/drm/tinydrm/Kconfig    |  10 ++
>   drivers/gpu/drm/tinydrm/Makefile   |   1 +
>   drivers/gpu/drm/tinydrm/wg160160.c | 298 +++++++++++++++++++++++++++++++++++++
>   4 files changed, 314 insertions(+)
>   create mode 100644 drivers/gpu/drm/tinydrm/wg160160.c

[...]

> +
> +/**
> + * write_reg - Write instruction on parallel bus to controller
> + *
> + * Check BUSY flag and write instruction
> + *
> + * @pdd: pardata data
> + * @reg: The register to write
> + * @value: The value of the register
> + *
> + * Returns:
> + * Zero on success, negative error code on failure
> + */
> +int write_reg(struct pardata_data *pdd, unsigned int reg, unsigned int value)
> +{
> +	int ins[PIN_NUM];
> +	int val[PIN_NUM];
> +	int i;
> +
> +	for (i = 0; i < PIN_NUM; i++)
> +		ins[PIN_DB0 + i] = !!BIT(reg);
> +
> +	for (i = 0; i < PIN_NUM; i++)
> +		val[PIN_DB0 + i] = !!(value & BIT(i));
> +
> +	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
> +	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
> +	wait_busy(pdd);
> +	pardata_strobe_write(pdd);
> +
> +	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
> +	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, val);
> +	wait_busy(pdd);
> +	pardata_strobe_write(pdd);
> +
> +	return 0;
> +}

If this controller has normal registers, you could do a regmap
implementation for pardata: drivers/base/regmap.

Noralf.

> +
> +/**
> + * write_buf - write buffer on parallel bus to controller
> + *
> + * @pdd: pardata data
> + * @offset: offset into display RAM
> + * @data: pointer to data to write
> + * @len: number of bytes to write
> + *
> + * Returns:
> + * Zero on success, negative error code on failure
> + */
> +int write_buf(struct pardata_data *pdd, u8 offset, u8 *data, size_t len)
> +{
> +	int ins[PIN_NUM];
> +	int val[PIN_NUM];
> +	int bit;
> +	int i;
> +
> +	/* Setup address */
> +	write_reg(pdd, WG160160_ADDRSL_REG, offset & 0xff);
> +	write_reg(pdd, WG160160_ADDRSL_REG, (offset >> 8) & 0xff);
> +
> +	/* prepare to write data */
> +	for (i = 0; i < PIN_NUM; i++)
> +		ins[PIN_DB0 + i] = !!(WG160160_WRITE_REG & BIT(i));
> +
> +	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
> +	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
> +	wait_busy(pdd);
> +	pardata_strobe_write(pdd);
> +
> +	/* Write data byte - by byte */
> +	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
> +
> +	for (i = offset; i < (offset + len); i++) {
> +		for (bit = 0; bit < PIN_NUM; bit++)
> +			val[PIN_DB0 + bit] = !!(data[i] & BIT(bit));
> +
> +		gpiod_set_array_value_cansleep(PIN_NUM,
> +					       pdd->bus->data_pins->desc,
> +					       val);
> +		wait_busy(pdd);
> +		pardata_strobe_write(pdd);
> +	}
> +
> +	return 0;
> +}