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.

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

drivers: misc: adi-axi-tdd: Fix reg values in ms #2603

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
24 changes: 18 additions & 6 deletions drivers/misc/adi-axi-tdd.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
#define ADI_REG_TDD_BURST_COUNT 0x004c
#define ADI_REG_TDD_STARTUP_DELAY 0x0050
#define ADI_REG_TDD_FRAME_LENGTH 0x0054
#define ADI_REG_TDD_SYNC_COUNTER_LOW 0x0058
#define ADI_REG_TDD_SYNC_COUNTER_HIGH 0x005c
#define ADI_REG_TDD_SYNC_PERIOD_LOW 0x0058
#define ADI_REG_TDD_SYNC_PERIOD_HIGH 0x005c
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why changing the name? If it makes more sense to have the new name please mention it in the commit message to avoid comments like this. Typically commits are small and address only one issue. Hence, if you#re doing some additional thing (even if small like this) please mention it. A couple of issues with your commit:

  1. Run git log --no-merges --oneline drivers/misc to see the used commit title in that dir. It seems to be a bit random but I would say the most used would be just misc: adi-axi-tdd: .... That said, what you have seems to be acceptable so up to you;
  2. You should describe and explain what you're fixing in the commit message. It helps in understanding what was the issue in the code (other than having to understand by looking at the code);
  3. You could also add a Fixes: tag. Not that important for ADI tree but it's a matter of keeping the same "routines" as upstream so it's easier then to send patches upstream.

#define ADI_REG_TDD_STATUS 0x0060
#define ADI_REG_TDD_CHANNEL_BASE 0x0080

Expand Down Expand Up @@ -233,6 +233,8 @@ static ssize_t adi_axi_tdd_show(struct device *dev,
ret = regmap_read(st->regs, ADI_REG_TDD_STARTUP_DELAY, &data);
if (ret)
return ret;
if (data)
data++;
return adi_axi_tdd_format_ms(st, data, buf);
case ADI_TDD_ATTR_FRAME_LENGTH_RAW:
ret = regmap_read(st->regs, ADI_REG_TDD_FRAME_LENGTH, &data);
Expand All @@ -243,18 +245,22 @@ static ssize_t adi_axi_tdd_show(struct device *dev,
ret = regmap_read(st->regs, ADI_REG_TDD_FRAME_LENGTH, &data);
if (ret)
return ret;
if (data)
data++;
return adi_axi_tdd_format_ms(st, data, buf);
case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW:
ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW,
ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_PERIOD_LOW,
&data64, 2);
if (ret)
return ret;
return sysfs_emit(buf, "%llu\n", data64);
case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS:
ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW,
ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_PERIOD_LOW,
&data64, 2);
if (ret)
return ret;
if (data64)
data64++;
return adi_axi_tdd_format_ms(st, data64, buf);
case ADI_TDD_ATTR_STATE:
ret = regmap_read(st->regs, ADI_REG_TDD_STATUS, &data);
Expand Down Expand Up @@ -406,6 +412,8 @@ static int adi_axi_tdd_write_regs(const struct adi_axi_tdd_attribute *attr,
return ret;
if (FIELD_GET(GENMASK_ULL(63, 32), data64))
return -EINVAL;
if (data64)
data64--;
return regmap_write(st->regs, ADI_REG_TDD_STARTUP_DELAY,
(u32)data64);
case ADI_TDD_ATTR_FRAME_LENGTH_RAW:
Expand All @@ -419,19 +427,23 @@ static int adi_axi_tdd_write_regs(const struct adi_axi_tdd_attribute *attr,
return ret;
if (FIELD_GET(GENMASK_ULL(63, 32), data64))
return -EINVAL;
if (data64)
data64--;
return regmap_write(st->regs, ADI_REG_TDD_FRAME_LENGTH,
(u32)data64);
case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW:
ret = kstrtou64(buf, 0, &data64);
if (ret)
return ret;
return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW,
return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_PERIOD_LOW,
&data64, 2);
case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS:
ret = adi_axi_tdd_parse_ms(st, buf, &data64);
if (ret)
return ret;
return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW,
if (data64)
data64--;
return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_PERIOD_LOW,
&data64, 2);
case ADI_TDD_ATTR_CHANNEL_ENABLE:
ret = kstrtou32(buf, 0, &data);
Expand Down