iio: dac: ad5592r: Create ADC_BUF_EN sysfs #2817
Conversation
Brian-Pellegrino-ARKA
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
drivers/iio/dac/ad5592r.c
Outdated
| if (ret) | ||
| return ret; | ||
|
|
||
| return scnprintf(buf, PAGE_SIZE, "%d\n", !!(reg & AD5592R_GEN_CTRL_ADC_BUF_EN)); |
There was a problem hiding this comment.
Replaced with sysfs_emit()
|
|
||
| ret = ad5592r_reg_write(st, AD5592R_REG_CTRL, reg); | ||
| if (ret) | ||
| return ret; |
There was a problem hiding this comment.
This is a RMW. Which means, a mutex is needed.
There was a problem hiding this comment.
Added mutex
drivers/iio/dac/ad5592r.c
Outdated
| static int ad5592r_spi_probe(struct spi_device *spi) | ||
| { | ||
| const struct spi_device_id *id = spi_get_device_id(spi); | ||
| int ret = ad5592r_probe(&spi->dev, id->name, &ad5592r_rw_ops); |
There was a problem hiding this comment.
Not typical kernel pattern calling a function like probe when declaring a variable.
There was a problem hiding this comment.
Fixed declarations to be kernel standard.
drivers/iio/dac/ad5592r.c
Outdated
| if (ret) | ||
| return ret; | ||
|
|
||
| struct iio_dev *indio = dev_get_drvdata(&spi->dev); |
There was a problem hiding this comment.
Also not linux coding style. No mixed code declarations.
There was a problem hiding this comment.
Fixed declarations to be kernel standard.
drivers/iio/dac/ad5592r.c
Outdated
|
|
||
| struct iio_dev *indio = dev_get_drvdata(&spi->dev); | ||
|
|
||
| ret = device_create_file(&indio->dev, &iio_dev_attr_adc_buf_en.dev_attr); |
There was a problem hiding this comment.
Also, in order to properly do this, you would likely need to change ad5592r_probe() so that you could pass this the device ATTR or an ATTR_GROUP down to the IIO register function.
There was a problem hiding this comment.
The attribute group is now passed to ad5592_probe() and handled there.
| return count; | ||
| } | ||
|
|
||
| static IIO_DEVICE_ATTR(adc_buf_en, 0644, adc_buf_en_show, adc_buf_en_store, 0); |
There was a problem hiding this comment.
This is non standard ABI. You'll have to justify why this is needed and what's the usecase for it.
There was a problem hiding this comment.
I added a justification and use case to the commit message.
Adds a read/write sysfs file for the ADC_BUF_EN bit of the GEN_CTRL_REG register ABI Change justification: Exposes the ADC_BUF_EN bit so user-space applications can dynamically enable or disable the AD5592R’s internal adc buffer at runtime, allowing switching between single-shot and buffered modes. ABI Change example use case: On board temperature readings are much more stable with the adc buffer enabled. Signed-off-by: Brian Pellegrino <bpellegrino@arka.org>
Brian-Pellegrino-ARKA
force-pushed
the
ad5592r_adc_buf_en_sysfs
branch
from
bd25925 to
40d23f1
Compare
|
I appreciate you taking a look through despite it needing to be upstreamed. I will make an upstream pull request. |
Add sysfs for ADC_BUF_EN
PR Description
Adds a read/write sysfs file for the
ADC_BUF_EN bit of the GEN_CTRL_REG register
PR Type
PR Checklist