On 19.09.2022 11:52, Vaittinen, Matti wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 9/9/22 11:12, Vaittinen, Matti wrote: >> Hi dee Ho peeps! >> >> Disclaimer - I have no HW to test this using real in-tree drivers. If >> someone has a device with a variant of bmc150 or adxl372 or - it'd be >> nice to see if reading hwfifo_watermark_max or hwfifo_watermark_min >> works with the v6.0-rc4. I've checked it on sama5d2_xplained board on v6.0 and it returns (null) for both hwfifo_watermark_max and hwfifo_watermark_min: # cat hwfifo_watermark_max (null) # cat hwfifo_watermark_min (null) With your series at [1] I have: # cat hwfifo_watermark_max 128 # cat hwfifo_watermark_min 2 Thank you, Claudiu Beznea [1] https://patchwork.kernel.org/project/linux-iio/list/?series=682707 > Maybe I am misreading code and have my own >> issues - in which case I apologize already now and go to the corner >> while being deeply ashamed :) > > I would like to add at least the at91-sama5d2_adc (conditonally > registers the IIO_CONST_ATTR for triggered-buffer) to the list of > devices that could be potentially tested. I hope some of these devices > had a user who could either make us worried and verify my assumption - > or make me ashamed but rest of us relieved :) Eg - I second my request > for testing this - and add potential owners of at91-sama5d2_adc to the list. > >> On 2/15/21 12:40, Alexandru Ardelean wrote: >>> This change wraps all buffer attributes into iio_dev_attr objects, and >>> assigns a reference to the IIO buffer they belong to. >>> >>> With the addition of multiple IIO buffers per one IIO device, we need a way >>> to know which IIO buffer is being enabled/disabled/controlled. >>> >>> We know that all buffer attributes are device_attributes. >> >> I think this assumption is slightly unsafe. I see few drivers adding >> IIO_CONST_ATTRs in attribute groups. For example the bmc150 and adxl372 >> add the hwfifo_watermark_min and hwfifo_watermark_max. >> > > and at91-sama5d2_adc > > //snip > >> I noticed that using >> IIO_CONST_ATTRs for triggered buffers seem to cause access to somewhere >> it shouldn't... Oops. >> >> Reading the code allows me to assume the problem is wrapping the >> attributes to IIO_DEV_ATTRs. >> >> static struct attribute *iio_buffer_wrap_attr(struct iio_buffer *buffer, >> + struct attribute *attr) >> +{ >> + struct device_attribute *dattr = to_dev_attr(attr); >> + struct iio_dev_attr *iio_attr; >> + >> + iio_attr = kzalloc(sizeof(*iio_attr), GFP_KERNEL); >> + if (!iio_attr) >> + return NULL; >> + >> + iio_attr->buffer = buffer; >> + memcpy(&iio_attr->dev_attr, dattr, sizeof(iio_attr->dev_attr)); >> >> This copy does assume all attributes are device_attrs, and does not take >> into account that IIO_CONST_ATTRS have the string stored in a struct >> iio_const_attr which is containing the dev_attr. Eg, copying in the >> iio_buffer_wrap_attr() does not copy the string - and later invoking the >> 'show' callback goes reading something else than the mentioned string >> because the pointer is not copied. > > Yours, > -- Matti > > -- > Matti Vaittinen > Linux kernel developer at ROHM Semiconductors > Oulu Finland > > ~~ When things go utterly wrong vim users can always type :help! ~~
