[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190919202420.GA523@renesas.com>
Date: Thu, 19 Sep 2019 20:24:33 +0000
From: Vincent Cheng <vincent.cheng.xh@...esas.com>
To: Andrew Lunn <andrew@...n.ch>
CC: "robh+dt@...nel.org" <robh+dt@...nel.org>,
"mark.rutland@....com" <mark.rutland@....com>,
"richardcochran@...il.com" <richardcochran@...il.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] ptp: Add a ptp clock driver for IDT ClockMatrix.
Hi Andrew,
On Wed, Sep 18, 2019 at 05:18:03PM EDT, Andrew Lunn wrote:
>On Wed, Sep 18, 2019 at 04:06:38PM -0400, vincent.cheng.xh@...esas.com wrote:
>
>> +static s32 idtcm_xfer(struct idtcm *idtcm,
>> + u8 regaddr,
>> + u8 *buf,
>> + u16 count,
>> + bool write)
>> +{
>> + struct i2c_client *client = idtcm->client;
>> + struct i2c_msg msg[2];
>> + s32 cnt;
>> +
>> + msg[0].addr = client->addr;
>> + msg[0].flags = 0;
>> + msg[0].len = 1;
>> + msg[0].buf = ®addr;
>> +
>> + msg[1].addr = client->addr;
>> + msg[1].flags = write ? 0 : I2C_M_RD;
>> + msg[1].len = count;
>> + msg[1].buf = buf;
>> +
>> + cnt = i2c_transfer(client->adapter, msg, 2);
>> +
>> + if (cnt < 0) {
>> + pr_err("i2c_transfer returned %d\n", cnt);
>
>dev_err(client->dev, "i2c_transfer returned %d\n", cnt);
>
>We then have an idea which device has a transfer error.
>
>Please try to not use pr_err() when you have some sort of device.
Sure thing, will replace pr_err() with dev_err().
>> +static s32 idtcm_state_machine_reset(struct idtcm *idtcm)
>> +{
>> + s32 err;
>> + u8 byte = SM_RESET_CMD;
>> +
>> + err = idtcm_write(idtcm, RESET_CTRL, SM_RESET, &byte, sizeof(byte));
>> +
>> + if (!err) {
>> + /* delay */
>> + set_current_state(TASK_INTERRUPTIBLE);
>> + schedule_timeout(_msecs_to_jiffies(POST_SM_RESET_DELAY_MS));
>
>Maybe use msleep_interruptable()?
Yes, will try using msleep_interruptable() and will replace if it works.
>> +static s32 idtcm_load_firmware(struct idtcm *idtcm,
>> + struct device *dev)
>> +{
>> + const struct firmware *fw;
>> + struct idtcm_fwrc *rec;
>> + u32 regaddr;
>> + s32 err;
>> + s32 len;
>> + u8 val;
>> + u8 loaddr;
>> +
>> + pr_info("requesting firmware '%s'\n", FW_FILENAME);
>
>dev_debug()
Thanks, will make the change.
>> +
>> + err = request_firmware(&fw, FW_FILENAME, dev);
>> +
>> + if (err)
>> + return err;
>> +
>> + pr_info("firmware size %zu bytes\n", fw->size);
>
>dev_debug()
>
>Maybe look through all your pr_info and downgrade most of them to
>dev_debug()
Yes, will go through and downgrade to dev_debug() accordingly.
Thanks,
Vincent
Powered by blists - more mailing lists