On Thu, 2019-07-18 at 13:50 +0100, Mark Brown wrote: > On Wed, Jul 17, 2019 at 02:51:06PM +0300, Alexandru Ardelean wrote: > > Some devices like the ADIS16460 IMU require a stall period between > > transfers, i.e. between when the CS is de-asserted and re-asserted. The > > default value of 10us is not enough. This change makes the delay > > configurable for when the next CS change goes active. > > To repeat my previous feedback: > > > This looks like cs_change_delay. Ack. Will use `cs_change_delay`. I have no strong preference regarding the name. > > Please use subject lines matching the style for the subsystem. This > makes it easier for people to identify relevant patches. Ack. Will look for SPI subsystem specific subject lines and use them. > > Please don't ignore review comments, people are generally making them > for a reason and are likely to have the same concerns if issues remain > unaddressed. Having to repeat the same comments can get repetitive and > make people question the value of time spent reviewing. If you disagree > with the review comments that's fine but you need to reply and discuss > your concerns so that the reviewer can understand your decisions. [ the following part should not be considered in a disrespectful tone ; the intent is nowhere near that, but text- communication has a design-flaw where a disrespectful tone may be interpreted [where there isn't one] ] My intent wasn't to ignore the review comment. Sorry if it came out like that. I assumed a patch re-spin was preferred vs a verbal discussion. Some people prefer patch re-spins as a basis for discussion. Various people have various preferences. Also, I wasn't sure how soon I'd get a reply back on this, since various people/maintainers have various reply-times. And I also [sometimes] have longer reply-back-times [which doesn't help either]. And I wasn't sure if `cs_change_delay` was fully intentional/ad-literam, or whether it was a feedback of the sorts "along-the-lines of `cs_change_delay`". While looking at `struct spi_transfer` the other "delay" fields seem to be: `word_delay_usecs` & `delay_usecs`, which is why I assumed `cs_change_delay_usecs` was preferred [though I will admit, it is a long var-name]. And the conclusion [from my side] is: maybe I rushed this a bit and maybe it annoyed you. Not my intention, and it'll take me a bit to adjust to your style. Moving forward. 1. I will use `cs_change_delay` as field name 2. I will use SPI subsystem subject line; I will admit I forget this stuff periodically Is there anything else I should consider? Or anything else to discuss? I'm open to elements I may have forgotten/omitted unintentionally. Thanks Alex
