media: tuner xc5000 firmware load fixes and improvements
cover media: tuner xc5000 firmware load fixes and improvements
This patch fixes firmware load issues seen during suspend and resume and speeds up firmware force load path. Releasing firmware from xc5000_release() instead of right after load helps avoid requesting firmware when it needs to be force loaded and fixes slowpath warnings during resume after suspend is done before firmware is loaded.
Shuah Khan (2):
media: tuner xc5000 - release firmwware from xc5000_release()
media: tuner xc5000 - try to avoid firmware load in resume path
drivers/media/tuners/xc5000.c | 50 ++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 15 deletions(-)
Signature · 1 line
1.7.10.4
PATCH 1/2 media: tuner xc5000 - release firmwware from xc5000_release()
xc5000 releases firmware right after loading it. Change it to save the firmware and release it from xc5000_release(). This helps avoid fecthing firmware when forced firmware load requests come in to change analog tv frequence and when firmware needs to be reloaded after suspend and resume.
--- drivers/media/tuners/xc5000.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)
drivers/media/tuners/xc5000.c +20 -14
@@ -70,6 +70,8 @@ struct xc5000_priv {struct dvb_frontend *fe;struct delayed_work timer_sleep; + +const struct firmware *firmware; };/* Misc Defines */
@@ -1136,20 +1138,23 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)if (!force &&xc5000_is_firmware_loaded (fe) ==0 )return 0 ; - ret =request_firmware (&fw, desired_fw->name, - priv->i2c_props.adap->dev.parent); -if (ret) { -printk (KERN_ERR "xc5000: Upload failed. (file not found?)\n" ); -return ret; - } - -dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size); +if (!priv->firmware) { + ret =request_firmware (&fw, desired_fw->name, + priv->i2c_props.adap->dev.parent); +if (ret) { +pr_err ("xc5000: Upload failed. rc %d\n" , ret); +return ret; + } +dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size); -if (fw->size != desired_fw->size) { -printk (KERN_ERR "xc5000: Firmware file with incorrect size\n" ); - ret = -EINVAL ; - goto err; - } +if (fw->size != desired_fw->size) { +pr_err ("xc5000: Firmware file with incorrect size\n" ); + ret = -EINVAL ; + goto err; + } + priv->firmware = fw; + }else + fw = priv->firmware;/* Try up to 5 times to load firmware */ for (i =0 ; i <5 ; i++) {
@@ -1232,7 +1237,6 @@ err:else printk (KERN_CONT " - too many retries. Giving up\n" ); -release_firmware (fw);return ret; }
@@ -1316,6 +1320,8 @@ static int xc5000_release(struct dvb_frontend *fe)if (priv) {cancel_delayed_work (&priv->timer_sleep);hybrid_tuner_release_state (priv); +if (priv->firmware) +release_firmware (priv->firmware); }mutex_unlock (&xc5000_list_mutex);
Signature · 1 line
1.7.10.4
Re: [PATCH 1/2] media: tuner xc5000 - release firmwware from xc5000_release()
Em Wed, 13 Aug 2014 19:09:23 -0600
Shuah Khan <[email protected]> escreveu:
Quoted 44 lines
xc5000 releases firmware right after loading it. Change it to save the firmware and release it from
xc5000_release(). This helps avoid fecthing firmware when forced firmware load requests come in to change analog tv frequence and when firmware needs to be reloaded after suspend and resume.Signed-off-by Shuah Khan <[email protected]>--- drivers/media/tuners/xc5000.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)drivers/media/tuners/xc5000.c +10
-8@@ -70,6 +70,8 @@ struct xc5000_priv {struct dvb_frontend *fe;struct delayed_work timer_sleep; + +const struct firmware *firmware; };/* Misc Defines */ @@ -1136,20 +1138,23 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)if (!force &&xc5000_is_firmware_loaded (fe) ==0 )return 0 ; - ret =request_firmware (&fw, desired_fw->name, - priv->i2c_props.adap->dev.parent); -if (ret) { -printk (KERN_ERR "xc5000: Upload failed. (file not found?)\n" ); -return ret; - } - -dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size); +if (!priv->firmware) { + ret =request_firmware (&fw, desired_fw->name, + priv->i2c_props.adap->dev.parent); +if (ret) { +pr_err ("xc5000: Upload failed. rc %d\n" , ret); +return ret; + } +dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size);
drivers/media/tuners/xc5000.c static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force) · line 1153
Quoted 10 lines
drivers/media/tuners/xc5000.c +4
-5-if (fw->size != desired_fw->size) { -printk (KERN_ERR "xc5000: Firmware file with incorrect size\n" ); - ret = -EINVAL ; - goto err; - } +if (fw->size != desired_fw->size) { +pr_err ("xc5000: Firmware file with incorrect size\n" ); + ret = -EINVAL ; + goto err;
In this case, we should be releasing the firmware too. Btw, this code will also add a memory leak, as priv->firmware will be null, but the firmware was loaded.
The rest of the patch looks ok.
Regards,
Mauro
Re: [PATCH 1/2] media: tuner xc5000 - release firmwware from xc5000_release()
Quoted 48 lines · Mauro Carvalho Chehab
Em Wed, 13 Aug 2014 19:09:23 -0600
Shuah Khan <[email protected]> escreveu:Quoted 45 lines
xc5000 releases firmware right after loading it. Change it to save the firmware and release it from
xc5000_release(). This helps avoid fecthing firmware when forced firmware load requests come in to change analog tv frequence and when firmware needs to be reloaded after suspend and resume.Signed-off-by Shuah Khan <[email protected]>--- drivers/media/tuners/xc5000.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)drivers/media/tuners/xc5000.c +10
-8@@ -70,6 +70,8 @@ struct xc5000_priv {struct dvb_frontend *fe;struct delayed_work timer_sleep; + +const struct firmware *firmware; };/* Misc Defines */ @@ -1136,20 +1138,23 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)if (!force &&xc5000_is_firmware_loaded (fe) ==0 )return 0 ; - ret =request_firmware (&fw, desired_fw->name, - priv->i2c_props.adap->dev.parent); -if (ret) { -printk (KERN_ERR "xc5000: Upload failed. (file not found?)\n" ); -return ret; - } - -dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size); +if (!priv->firmware) { + ret =request_firmware (&fw, desired_fw->name, + priv->i2c_props.adap->dev.parent); +if (ret) { +pr_err ("xc5000: Upload failed. rc %d\n" , ret); +return ret; + } +dprintk (1 ,"firmware read %Zu bytes.\n" , fw->size);
Quoted 13 lines
drivers/media/tuners/xc5000.c static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force) · line 1153
Quoted 9 lines
drivers/media/tuners/xc5000.c +4
-5-if (fw->size != desired_fw->size) { -printk (KERN_ERR "xc5000: Firmware file with incorrect size\n" ); - ret = -EINVAL ; - goto err; - } +if (fw->size != desired_fw->size) { +pr_err ("xc5000: Firmware file with incorrect size\n" ); + ret = -EINVAL ; + goto err;In this case, we should be releasing the firmware too. Btw, this code will also add a memory leak, as
priv->firmwarewill be null, but the firmware was loaded.
Yes you are right. I will revise the patch series and resend it.
-- Shuah
Signature · 4 lines
Shuah Khan Sr. Linux Kernel Developer Samsung Research America (Silicon Valley) [email protected] | (970) 217-8978
PATCH 2/2 media: tuner xc5000 - try to avoid firmware load in resume path
In reply to Shuah Khanxc5000 doesn't load firmware at attach time instead loads it when it needs to set and change configuration from its init, frequency, digital and analog mode set interffaces. As a result, when system is suspended before firmware is loaded, firmware load can be avoided during resume. Loading formware in this scenario results in slowpath warnings during resume as it won't be in the suspend firmware cache.
--- drivers/media/tuners/xc5000.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/media/tuners/xc5000.c +15 -1
@@ -1293,6 +1293,20 @@ static int xc5000_suspend(struct dvb_frontend *fe)return 0 ; } +static int xc5000_resume (struct dvb_frontend *fe) +{ +struct xc5000_priv *priv = fe->tuner_priv; + +dprintk (1 ,"%s()\n" , __func__); + +/* suspended before firmware is loaded. +Avoid firmware load in resume path. */ +if (!priv->firmware) +return 0 ; + +return xc5000_set_params (fe); +} +static int xc5000_init (struct dvb_frontend *fe) {struct xc5000_priv *priv = fe->tuner_priv;
@@ -1360,7 +1374,7 @@ static const struct dvb_tuner_ops xc5000_tuner_ops = { .init = xc5000_init, .sleep = xc5000_sleep, .suspend = xc5000_suspend, - .resume = xc5000_set_params, + .resume = xc5000_resume, .set_config = xc5000_set_config, .set_params = xc5000_set_digital_params,
Signature · 1 line
1.7.10.4