On Fri, May 08, 2015 at 12:40:49AM +0800, Fu Wei wrote:
Hi Guenter,
On 5 May 2015 at 00:36, Guenter Roeck linux@roeck-us.net wrote:
On Tue, May 05, 2015 at 12:03:05AM +0800, Fu Wei wrote:
Hi Guenter,
Great thanks for your feedback, will fix them ASAP. Some feedback and comment inline below :
[ ... ]
+static int sbsa_gwdt_stop(struct watchdog_device *wdd) +{
struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);struct sbsa_gwdt_device_data *device_data =&gwdt->device_data;
u32 status;mutex_lock(&device_data->lock);status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);status &= ~SBSA_GWDT_WCS_EN;/* writing WCS will cause an explicit watchdog refresh */sbsa_gwdt_cf_write(SBSA_GWDT_WCS, status, wdd);status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);mutex_unlock(&device_data->lock);/* Check if the watchdog was disabled */if (status & SBSA_GWDT_WCS_EN)return -EACCES;Same as above.
For these two, maybe EPERM?
"Operation not permitted" seems just as wrong. Both usually reflect a software state, not a failure to write to hardware.
The question here really is if this can really happen, and, if it does, why it happens. In general it is quite uncommon for watchdog drivers to verify if such writes were successful.
I think , EIO is the best one, we config the reg to enable it , but fail, the mean we have problem on bus/IO, Do you agree?
Ok. Question though is still if this is a realistic error. In other words, is there any reason to believe that this can happen ? Because it is not really common in the kernel to read back a value from an io register unless there is reason to believe that a write can fail. Or, in other words, io writes are commonly seen to be reliable unless there is reason to believe otherwise.
Thanks, Guenter