Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Miscellaneous HD44780 auxdisplay driver enhancements #78590

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Miscellaneous HD44780 auxdisplay driver enhancements #78590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e89a60
drivers: auxdisplay: hd44780: set rw_gpio line value only once
kuliga Aug 27, 2024
f3ce47a
drivers: auxdisplay: hd44780: rework auxdisplay_hd44780_command()
kuliga Sep 13, 2024
fc6dd62
drivers: auxdisplay: hd44780: get rid of excessive delays
kuliga Sep 1, 2024
8d752bc
drivers: auxdisplay: hd44780: add rs-line-delay dt parameter
kuliga Sep 1, 2024
c5060be
drivers: auxdisplay: hd44780: rework the initialization procedure
kuliga Sep 13, 2024
9679e68
drivers: auxdisplay: hd44780: implement busy-flag polling mechanism
kuliga Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 136 additions & 59 deletions drivers/auxdisplay/auxdisplay_hd44780.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,53 +70,151 @@
uint8_t line_addresses[4];
uint16_t enable_line_rise_delay;
uint16_t enable_line_fall_delay;
uint16_t rs_line_delay;
uint16_t clear_delay;
uint16_t boot_delay;
};

static void auxdisplay_hd44780_set_entry_mode(const struct device *dev);
static void auxdisplay_hd44780_set_display_mode(const struct device *dev, bool enabled);
static int auxdisplay_hd44780_clear(const struct device *dev);

static void auxdisplay_hd44780_command(const struct device *dev, bool rs, uint8_t cmd,
uint8_t mode)
static void hd44780_pulse_enable_line(const struct device *dev)
{
const struct auxdisplay_hd44780_config *const config = dev->config;

gpio_pin_set_dt(&config->e_gpio, 1);
k_sleep(K_NSEC(config->enable_line_rise_delay));
gpio_pin_set_dt(&config->e_gpio, 0);
k_sleep(K_NSEC(config->enable_line_fall_delay));
}

static inline void hd44780_set_rs_rw_lines(const struct device *dev, bool rs, bool rw)
{
const struct auxdisplay_hd44780_config *const config = dev->config;

gpio_pin_set_dt(&config->rs_gpio, rs);
if (config->rw_gpio.port) {
gpio_pin_set_dt(&config->rw_gpio, rw);
}

k_sleep(K_NSEC(config->rs_line_delay));
}

static int hd44780_db_gpios_configure(const struct device *dev, uint8_t lsb_line,
gpio_flags_t flags)
{
const struct auxdisplay_hd44780_config *config = dev->config;
int rc;

for (int line = 7; line >= lsb_line; --line) {
rc = gpio_pin_configure_dt(&config->db_gpios[line], flags);
if (rc < 0) {
return rc;
}
}

return 0;
}

static void auxdisplay_hd44780_command(const struct device *dev, bool rs,
uint8_t cmd, uint8_t mode)
{
int rc;
const struct auxdisplay_hd44780_config *config = dev->config;
int8_t i = 7;
const int8_t lsb_line = (mode == AUXDISPLAY_HD44780_MODE_8_BIT) ? 0 : 4;
int8_t ncommands = (mode == AUXDISPLAY_HD44780_MODE_4_BIT) ? 2 : 1;
const bool check_busy_flag = (!config->rw_gpio.port ||
(mode == AUXDISPLAY_HD44780_MODE_4_BIT_ONCE)) ?
false : true;

Check notice on line 131 in drivers/auxdisplay/auxdisplay_hd44780.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/auxdisplay/auxdisplay_hd44780.c:131 
-static void auxdisplay_hd44780_command(const struct device *dev, bool rs,
-				       uint8_t cmd, uint8_t mode)
+static void auxdisplay_hd44780_command(const struct device *dev, bool rs, uint8_t cmd, uint8_t mode)
 {
 	int rc;
 	const struct auxdisplay_hd44780_config *config = dev->config;
 	int8_t i = 7;
 	const int8_t lsb_line = (mode == AUXDISPLAY_HD44780_MODE_8_BIT) ? 0 : 4;
 	int8_t ncommands = (mode == AUXDISPLAY_HD44780_MODE_4_BIT) ? 2 : 1;
-	const bool check_busy_flag = (!config->rw_gpio.port ||
-				      (mode == AUXDISPLAY_HD44780_MODE_4_BIT_ONCE)) ?
-				      false : true;
+	const bool check_busy_flag =
+		(!config->rw_gpio.port || (mode == AUXDISPLAY_HD44780_MODE_4_BIT_ONCE)) ? false
+											: true;
if (mode == AUXDISPLAY_HD44780_MODE_8_BIT) {
while (i >= 0) {
gpio_pin_set_dt(&config->db_gpios[i], ((cmd & BIT(i)) ? 1 : 0));
--i;
if (check_busy_flag) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

section should probably just be #ifdef'd out if no instances support it though can defer to a later PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, actually when I first started to implement it, I wanted this section to be #ifdef'd exactly as you mentioned. The thing is though, that I thinks it is not necessary. In case where the user doesn't specify this option, it won't affect him at all, since the !config->rw_gpio.port test is going to be resolved at compile time (this config struct is defined as static const, its members values come from dt and are resolved during compile-time). That's why I decided to do it this way.

bool busy;

rc = hd44780_db_gpios_configure(dev, lsb_line, GPIO_INPUT | GPIO_PULL_DOWN);
if (rc < 0) {
LOG_ERR("configuration of db-gpios as inputs failed: %d", rc);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration (with capital C)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

return;
}
} else {
while (i >= 4) {
gpio_pin_set_dt(&config->db_gpios[i], ((cmd & BIT(i)) ? 1 : 0));

hd44780_set_rs_rw_lines(dev, 0, 1);
do {
hd44780_pulse_enable_line(dev);

/* We don't care about the other pins. */
busy = gpio_pin_get_dt(&config->db_gpios[7]);

if (config->capabilities.mode == AUXDISPLAY_HD44780_MODE_4_BIT) {
/* In this mode we have to initiate two separate readbacks. */
hd44780_pulse_enable_line(dev);
}
} while (busy);

rc = hd44780_db_gpios_configure(dev, lsb_line, GPIO_OUTPUT);
if (rc < 0) {
LOG_ERR("configuration of db-gpios as outputs failed: %d", rc);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

return;
}
}

hd44780_set_rs_rw_lines(dev, rs, 0);
while (ncommands--) {
for (int8_t line = 7; line >= lsb_line; --line) {
gpio_pin_set_dt(&config->db_gpios[line], ((cmd & BIT(i)) ? 1 : 0));
--i;
}

hd44780_pulse_enable_line(dev);
}

gpio_pin_set_dt(&config->rs_gpio, rs);
if (!check_busy_flag) {
/* Sleep for a max execution time for a given instruction. */
uint16_t cmd_delay_us = (cmd == AUXDISPLAY_HD44780_CMD_CLEAR) ? 1520 : 37;

if (config->rw_gpio.port) {
gpio_pin_set_dt(&config->rw_gpio, 0);
k_sleep(K_USEC(cmd_delay_us));
}
}

gpio_pin_set_dt(&config->e_gpio, 1);
k_sleep(K_USEC(config->enable_line_rise_delay));
gpio_pin_set_dt(&config->e_gpio, 0);
k_sleep(K_USEC(config->enable_line_fall_delay));
static void hd44780_ic_initialize(const struct device *dev)
{
const struct auxdisplay_hd44780_config *config = dev->config;
uint8_t cmd;

/*
* If proper power supply is used to power the HD44780, it initializes correctly
* on a reset condition all by itself. However, if the power supply is below
* its expectations (e.g. supplying it with some 3.3V Nucleo board),
* it won't initialize properly on its own, and the MCU has to carry out
* the initialization as listed in the reference manual.
* Since we cannot determine it properly in the runtime,
* always carry out the initialization procedure.
*/
cmd = AUXDISPLAY_HD44780_CMD_SETUP | AUXDISPLAY_HD44780_8_BIT_CONFIG;
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
k_sleep(K_USEC(4100));
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
k_sleep(K_USEC(100));
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
k_sleep(K_USEC(100));

if (mode == AUXDISPLAY_HD44780_MODE_4_BIT) {
while (i >= 0) {
gpio_pin_set_dt(&config->db_gpios[(i + 4)], ((cmd & BIT(i)) ? 1 : 0));
--i;
}
if (config->capabilities.mode == AUXDISPLAY_HD44780_MODE_4_BIT) {
/* Put display into 4-bit mode */
cmd = AUXDISPLAY_HD44780_CMD_SETUP;
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
}

gpio_pin_set_dt(&config->e_gpio, 1);
k_sleep(K_USEC(config->enable_line_rise_delay));
gpio_pin_set_dt(&config->e_gpio, 0);
k_sleep(K_USEC(config->enable_line_fall_delay));
/* Configure display */
if (config->capabilities.rows > 1) {
cmd |= AUXDISPLAY_HD44780_2_LINE_CONFIG;
}
auxdisplay_hd44780_command(dev, false, cmd, config->capabilities.mode);

auxdisplay_hd44780_set_display_mode(dev, false);
auxdisplay_hd44780_clear(dev);
auxdisplay_hd44780_set_entry_mode(dev);

auxdisplay_hd44780_set_display_mode(dev, true);
}

static int auxdisplay_hd44780_init(const struct device *dev)
Expand All @@ -125,7 +223,6 @@
struct auxdisplay_hd44780_data *data = dev->data;
int rc;
uint8_t i = 0;
uint8_t cmd = AUXDISPLAY_HD44780_CMD_SETUP | AUXDISPLAY_HD44780_8_BIT_CONFIG;

if (config->capabilities.mode > AUXDISPLAY_HD44780_MODE_8_BIT) {
/* This index is reserved for internal driver usage */
Expand All @@ -141,15 +238,6 @@
return rc;
}

if (config->rw_gpio.port) {
rc = gpio_pin_configure_dt(&config->rw_gpio, GPIO_OUTPUT);

if (rc < 0) {
LOG_ERR("Configuration of RW GPIO failed: %d", rc);
return rc;
}
}

rc = gpio_pin_configure_dt(&config->e_gpio, GPIO_OUTPUT);

if (rc < 0) {
Expand Down Expand Up @@ -182,6 +270,15 @@
++i;
}

if (config->rw_gpio.port) {
rc = gpio_pin_configure_dt(&config->rw_gpio, GPIO_OUTPUT);

if (rc < 0) {
LOG_ERR("Configuration of RW GPIO failed: %d", rc);
return rc;
}
}

if (config->backlight_gpio.port) {
rc = gpio_pin_configure_dt(&config->backlight_gpio, GPIO_OUTPUT);

Expand All @@ -207,28 +304,7 @@
k_sleep(K_MSEC(config->boot_delay));
}

if (config->capabilities.mode == AUXDISPLAY_HD44780_MODE_4_BIT) {
/* Reset display to known state in 8-bit mode */
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);

/* Put display into 4-bit mode */
cmd = AUXDISPLAY_HD44780_CMD_SETUP;
auxdisplay_hd44780_command(dev, false, cmd, AUXDISPLAY_HD44780_MODE_4_BIT_ONCE);
}

if (config->capabilities.rows > 1) {
cmd |= AUXDISPLAY_HD44780_2_LINE_CONFIG;
}

/* Configure display */
auxdisplay_hd44780_command(dev, false, cmd, config->capabilities.mode);
auxdisplay_hd44780_set_display_mode(dev, true);
auxdisplay_hd44780_set_entry_mode(dev);
auxdisplay_hd44780_command(dev, false, AUXDISPLAY_HD44780_CMD_CLEAR,
config->capabilities.mode);

k_sleep(K_USEC(config->clear_delay));
hd44780_ic_initialize(dev);

return 0;
}
Expand Down Expand Up @@ -577,8 +653,9 @@
.line_addresses[2] = DT_INST_PROP_BY_IDX(inst, line_addresses, 2), \
.line_addresses[3] = DT_INST_PROP_BY_IDX(inst, line_addresses, 3), \
.backlight_gpio = GPIO_DT_SPEC_INST_GET_OR(inst, backlight_gpios, {0}), \
.enable_line_rise_delay = DT_INST_PROP(inst, enable_line_rise_delay_us), \
.enable_line_fall_delay = DT_INST_PROP(inst, enable_line_fall_delay_us), \
.enable_line_rise_delay = DT_INST_PROP(inst, enable_line_rise_delay_ns), \
.enable_line_fall_delay = DT_INST_PROP(inst, enable_line_fall_delay_ns), \
.rs_line_delay = DT_INST_PROP(inst, rs_line_delay_ns), \
.clear_delay = DT_INST_PROP(inst, clear_command_delay_us), \
.boot_delay = DT_INST_PROP(inst, boot_delay_ms), \
}; \
Expand All @@ -590,5 +667,5 @@
POST_KERNEL, \
CONFIG_AUXDISPLAY_INIT_PRIORITY, \
&auxdisplay_hd44780_auxdisplay_api);

Check notice on line 670 in drivers/auxdisplay/auxdisplay_hd44780.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/auxdisplay/auxdisplay_hd44780.c:670 
-#define AUXDISPLAY_HD44780_DEVICE(inst)								\
-	static struct auxdisplay_hd44780_data auxdisplay_hd44780_data_##inst;			\
-	static const struct auxdisplay_hd44780_config auxdisplay_hd44780_config_##inst = {	\
-		.capabilities = {								\
-			.columns = DT_INST_PROP(inst, columns),					\
-			.rows = DT_INST_PROP(inst, rows),					\
-			.mode = DT_INST_ENUM_IDX(inst, mode),					\
-			.brightness.minimum = AUXDISPLAY_LIGHT_NOT_SUPPORTED,			\
-			.brightness.maximum = AUXDISPLAY_LIGHT_NOT_SUPPORTED,			\
-			.backlight.minimum = BACKLIGHT_CHECK(inst,				\
-							     AUXDISPLAY_HD44780_BACKLIGHT_MIN),	\
-			.backlight.maximum = BACKLIGHT_CHECK(inst,				\
-							     AUXDISPLAY_HD44780_BACKLIGHT_MAX),	\
-			.custom_characters = AUXDISPLAY_HD44780_CUSTOM_CHARACTERS,		\
-			.custom_character_width = AUXDISPLAY_HD44780_CUSTOM_CHARACTER_WIDTH,	\
-			.custom_character_height = AUXDISPLAY_HD44780_CUSTOM_CHARACTER_HEIGHT,	\
-		},										\
-		.rs_gpio = GPIO_DT_SPEC_INST_GET(inst, register_select_gpios),			\
-		.rw_gpio = GPIO_DT_SPEC_INST_GET_OR(inst, read_write_gpios, {0}),		\
-		.e_gpio = GPIO_DT_SPEC_INST_GET(inst, enable_gpios),				\
-		.db_gpios[0] = GPIO_DT_SPEC_INST_GET_BY_IDX_OR(inst, data_bus_gpios, 0, {0}),	\
-		.db_gpios[1] = GPIO_DT_SPEC_INST_GET_BY_IDX_OR(inst, data_bus_gpios, 1, {0}),	\
-		.db_gpios[2] = GPIO_DT_SPEC_INST_GET_BY_IDX_OR(inst, data_bus_gpios, 2, {0}),	\
-		.db_gpios[3] = GPIO_DT_SPEC_INST_GET_BY_IDX_OR(inst, data_bus_gpios, 3, {0}),	\
-		.db_gpios[4] = GPIO_DT_SPEC_INST_GET_BY_IDX(inst, data_bus_gpios, 4),		\
-		.db_gpios[5] = GPIO_DT_SPEC_INST_GET_BY_IDX(inst, data_bus_gpios, 5),		\
-		.db_gpios[6] = GPIO_DT_SPEC_INST_GET_BY_IDX(inst, data_bus_gpios, 6),		\
-		.db_gpios[7] = GPIO_DT_SPEC_INST_GET_BY_IDX(inst, data_bus_gpios, 7),		\
-		.line_addresses[0] = DT_INST_PROP_BY_IDX(inst, line_addresses, 0),		\
-		.line_addresses[1] = DT_INST_PROP_BY_IDX(inst, line_addresses, 1),		\
-		.line_addresses[2] = DT_INST_PROP_BY_IDX(inst, line_addresses, 2),		\
-		.line_addresses[3] = DT_INST_PROP_BY_IDX(inst, line_addresses, 3),		\
-		.backlight_gpio = GPIO_DT_SPEC_INST_GET_OR(inst, backlight_gpios, {0}),		\
-		.enable_line_rise_delay = DT_INST_PROP(inst, enable_line_rise_delay_ns),	\
-		.enable_line_fall_delay = DT_INST_PROP(inst, enable_line_fall_delay_ns),	\
-		.rs_line_delay = DT_INST_PROP(inst, rs_line_delay_ns),				\
-		.clear_delay = DT_INST_PROP(inst, clear_command_delay_us),			\
-		.boot_delay = DT_INST_PROP(inst, boot_delay_ms),				\
-	};											\
-	DEVICE_DT_INST_DEFINE(inst,								\
-			&auxdisplay_hd44780_init,						\
-			NULL,									\
-			&auxdisplay_hd44780_data_##inst,					\
-			&auxdisplay_hd44780_config_##inst,					\
-			POST_KERNEL,								\
-			CONFIG_AUXDISPLAY_INIT_PRIORITY,					\
-			&auxdisplay_hd44780_auxdisplay_api);
+#define AUXDISPLAY_HD44780_DEVICE(inst)                                                            \
+	static struct auxdisplay_hd44780_data auxdisplay_hd44780_data_##inst;                      \
+	static const struct auxdisplay_hd44780_config auxdisplay_hd44780_config_##inst = {         \
+		.capabilities =                                                                    \
+			{                                                                          \
+				.columns = DT_INST_PROP(inst, columns),                            \
+				.rows = DT_INST_PROP(inst, rows),                                  \
+				.mode = DT_INST_ENUM_IDX(inst, mode),                              \
+				.brightness.minimum = AUXDISPLAY_LIGHT_NOT_SUPPORTED,              \
+				.brightness.maximum = AUXDISPLAY_LIGHT_NOT_SUPPORTED,              \
+				.backlight.minimum =                                               \
+					BACKLIGHT_CHECK(inst, AUXDISPLAY_HD44780_BACKLIGHT_MIN),   \
+				.backlight.maximum =                                               \
+					BACKLIGHT_CHECK(inst, AUXDISPLAY_HD44780_BACKLIGHT_MAX),   \
+				.custom_characters = AUXDISPLAY_HD44780_CUSTOM_CHARACTERS,         \
+				.custom_character_width =
DT_INST_FOREACH_STATUS_OKAY(AUXDISPLAY_HD44780_DEVICE)
19 changes: 13 additions & 6 deletions dts/bindings/auxdisplay/hit,hd44780.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,27 @@ properties:
Array of addresses for each row, will use defaults if not provided.
Default is as per Hitachi HD44780 specification.

enable-line-rise-delay-us:
enable-line-rise-delay-ns:
type: int
default: 800
default: 450
description: |
Delay time (in us) to wait after enable line rises before setting low.
Delay time (in ns) to wait after enable line rises before setting low.
Default is as per Hitachi HD44780 specification.

enable-line-fall-delay-us:
enable-line-fall-delay-ns:
type: int
default: 100
default: 550
description: |
Delay time (in us) to wait after enable line falls before sending
Delay time (in ns) to wait after enable line falls before sending
another command. Default is as per Hitachi HD44780 specification.

rs-line-delay-ns:
type: int
default: 60
description: |
Delay time (in ns) to wait after rs/rw line state has been set up before
sending another command. Default is as per Hitachi HD44780 specification.

clear-command-delay-us:
type: int
default: 5000
Expand Down
Loading