Hi Jacopo, Sakari,
As you want, let's drop this patch.
BR,
Hugues.
On 10/7/20 5:09 PM, Jacopo Mondi wrote:
> Hi Hugues, Sakari,
>
> On Wed, Oct 07, 2020 at 06:01:03PM +0300, Sakari Ailus wrote:
>> Hi Hugues,
>>
>> On Wed, Oct 07, 2020 at 04:43:18PM +0200, Hugues Fruchet wrote:
>>> fixes: b1751ae652fb ("media: i2c: ov5640: Separate out mipi configuration from s_power")
>>
>> Fixes: ...
>>
>> And preferrably before Sob line.
>>
>>>
>>> Fix ov5640_write_reg()return value unchecked at power off.
>>> Reformat code to keep register access below the register description.
>>
>> If there's an error, I wouldn't stop turning things off, but just proceed.
>> The caller will ignore the error in that case anyway. This changes with the
>> patch.
>
> Agreed, I think it's best to continue instead of bailing out as we're
> in a power-off path
>
>>
>>>
>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxx>
>>> Change-Id: I12db67c416c3d63eadee400a3c89aaf48c5b1469
>>
>> This was probably not intended to be here.
>>
>>> ---
>>> drivers/media/i2c/ov5640.c | 17 ++++++-----------
>>> 1 file changed, 6 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>>> index 8d0254d..f34e62e 100644
>>> --- a/drivers/media/i2c/ov5640.c
>>> +++ b/drivers/media/i2c/ov5640.c
>>> @@ -1940,14 +1940,6 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>>> {
>>> int ret;
>>>
>>> - if (!on) {
>>> - /* Reset MIPI bus settings to their default values. */
>>> - ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x58);
>>> - ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x04);
>>> - ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x00);
>>> - return 0;
>>> - }
>>> -
>>> /*
>>> * Power up MIPI HS Tx and LS Rx; 2 data lanes mode
>>> *
>>> @@ -1958,7 +1950,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>>> * [3] = 0 : Power up MIPI LS Rx
>>> * [2] = 0 : MIPI interface disabled
>>> */
>>> - ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, 0x40);
>>> + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
>>> + on ? 0x40 : 0x58);
>>> if (ret)
>>> return ret;
>>>
>>> @@ -1969,7 +1962,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>>> * [5] = 1 : Gate clock when 'no packets'
>>> * [2] = 1 : MIPI bus in LP11 when 'no packets'
>>> */
>>> - ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00, 0x24);
>>> + ret = ov5640_write_reg(sensor, OV5640_REG_MIPI_CTRL00,
>>> + on ? 0x24 : 0x04);
>>> if (ret)
>>> return ret;
>>>
>>> @@ -1981,7 +1975,8 @@ static int ov5640_set_power_mipi(struct ov5640_dev *sensor, bool on)
>>> * [5] = 1 : MIPI data lane 1 in LP11 when 'sleeping'
>>> * [4] = 1 : MIPI clock lane in LP11 when 'sleeping'
>>> */
>>> - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, 0x70);
>>> + ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
>>> + on ? 0x70 : 0x00);
>>> if (ret)
>>> return ret;
>>>
>>
>> --
>> Kind regards,
>>
>> Sakari Ailus
