On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> +static int cpld_reg_update(struct up_board_cpld *cpld)
> +{
> + u64 dir_reg_verify = 0;
> + int i;
> +
> + /* Reset the CPLD internal counters */
> + gpiod_set_value(cpld->reset_gpio.soc_gpiod, 0);
> + gpiod_set_value(cpld->reset_gpio.soc_gpiod, 1);
> +
> + /*
> + * Update the CPLD dir register
> + * data_in will be sampled on each rising edge of the strobe
> signal
> + */
> + for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> + gpiod_set_value(cpld->data_in_gpio.soc_gpiod,
> + (cpld->dir_reg >> i) & 0x1);
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> + }
> +
> + /*
> + * Read back and verify the value
> + * data_out will be set on each rising edge of the strobe
> signal
> + */
> + for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
> + int data_out;
> +
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> + data_out = gpiod_get_value(cpld-
> >data_out_gpio.soc_gpiod);
> + dir_reg_verify |= (u64)data_out << i;
> + }
> +
> + if (dir_reg_verify != cpld->dir_reg) {
> + pr_err("CPLD verify error (expected: %llX, actual:
> %llX)\n",
> + cpld->dir_reg, dir_reg_verify);
dev_err();
> + return -EIO;
> + }
> +
> + /* Issue a dummy STB cycle to latch the dir register updates
> */
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> + gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> +
> + return 0;
> +}
> +
> +/**
> + * up_board_cpld_reg_set_bit() - update CPLD configuration
> + * @cpld: CPLD internal context info reference
> + * @offset: bit offset in CPLD register to set
> + * @value: boolean value to set in CPLD register bit selected
> by offset
> + *
> + * Return: Returns 0 if successful, or negative error value
> otherwise
> + */
> +static int up_board_cpld_reg_set_bit(struct up_board_cpld *cpld,
> + unsigned int offset, int value)
> +{
> + u64 old_regval;
> + int ret = 0;
> +
> + spin_lock(&cpld->lock);
> +
> + old_regval = cpld->dir_reg;
> +
> + if (value)
> + cpld->dir_reg |= 1ULL << offset;
> + else
> + cpld->dir_reg &= ~(1ULL << offset);
> +
> + /* Only update the CPLD register if it has changed */
> + if (cpld->dir_reg != old_regval)
> + ret = cpld_reg_update(cpld);
> +
> + spin_unlock(&cpld->lock);
Seems to me as though cpld_reg_update() could be quite lengthy. Would a
mutex be a better choice here ?
> +static int up_board_cpld_setup(struct up_board_cpld *cpld)
> +{
> + struct up_board_gpio_info *cpld_gpios[] = {
> + &cpld->strobe_gpio,
> + &cpld->reset_gpio,
> + &cpld->data_in_gpio,
> + &cpld->data_out_gpio,
> + &cpld->oe_gpio,
> + };
> + int i, ret;
> +
> + spin_lock_init(&cpld->lock);
> +
> + /* Initialise the CPLD config input GPIOs as outputs,
> initially low */
> + for (i = 0; i < ARRAY_SIZE(cpld_gpios); i++) {
> + struct up_board_gpio_info *gpio = cpld_gpios[i];
> +
> + ret = up_board_soc_gpio_setup(cpld, gpio);
> + if (ret)
> + return ret;
> +
> + ret = devm_gpio_request_one(cpld->dev, gpio-
> >soc_gpio,
> + gpio->soc_gpio_flags,
> + dev_name(cpld->dev));
> + if (ret)
> + return ret;
> + }
> +
> + /* Load initial CPLD configuration (all pins set for GPIO
> input) */
> + ret = cpld_reg_update(cpld);
> + if (ret) {
devm_gpio_free() ?
---
bod
