[PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Evgeny Novikov]

It seems that mxic_nfc_probe() missed invocation of
mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
was refined appropriately.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
---
 drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_nand.c
index da1070993994..37e75bf60ee5 100644
--- a/drivers/mtd/nand/raw/mxic_nand.c
+++ b/drivers/mtd/nand/raw/mxic_nand.c
@@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device *pdev)
 	if (IS_ERR(nfc->send_dly_clk))
 		return PTR_ERR(nfc->send_dly_clk);
 
+	err = mxic_nfc_clk_enable(nfc);
+	if (err)
+		return err;
+
 	nfc->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(nfc->regs))
-		return PTR_ERR(nfc->regs);
+	if (IS_ERR(nfc->regs)) {
+		err = PTR_ERR(nfc->regs);
+		goto fail;
+	}
 
 	nand_chip = &nfc->chip;
 	mtd = nand_to_mtd(nand_chip);
@@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device *pdev)
 	nand_chip->controller = &nfc->controller;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	if (irq < 0) {
+		err = irq;
+		goto fail;
+	}
 
 	mxic_nfc_hw_init(nfc);
 
-- 
2.26.2

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Miquel Raynal]

Hi Andy,

Andy Shevchenko <andy.shevchenko@gmail.com> wrote on Thu, 12 Aug 2021
15:13:10 +0300:

> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru> wrote:
> 
> > It seems that mxic_nfc_probe() missed invocation of
> > mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
> > was refined appropriately.  
> 
> 
> NAK. Until you provide a deeper analysis, like how this works before your
> change.
> 
> 
> Please, don’t blindly generate patches, this can even your bot do, just
> think about each change and preferable test on the real hardware.
> 
> The above is to all your lovely contributions.
> 
> 
> >
> > Found by Linux Driver Verification project (linuxtesting.org).
> >
> > Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
> > Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> > Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> > ---
> >  drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_
> > nand.c
> > index da1070993994..37e75bf60ee5 100644
> > --- a/drivers/mtd/nand/raw/mxic_nand.c
> > +++ b/drivers/mtd/nand/raw/mxic_nand.c
> > @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device
> > *pdev)
> >         if (IS_ERR(nfc->send_dly_clk))
> >                 return PTR_ERR(nfc->send_dly_clk);
> >
> > +       err = mxic_nfc_clk_enable(nfc);
> > +       if (err)
> > +               return err;

As Andy said, this is not needed.

> > +
> >         nfc->regs = devm_platform_ioremap_resource(pdev, 0);
> > -       if (IS_ERR(nfc->regs))
> > -               return PTR_ERR(nfc->regs);
> > +       if (IS_ERR(nfc->regs)) {
> > +               err = PTR_ERR(nfc->regs);
> > +               goto fail;
> > +       }
> >
> >         nand_chip = &nfc->chip;
> >         mtd = nand_to_mtd(nand_chip);
> > @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device
> > *pdev)
> >         nand_chip->controller = &nfc->controller;
> >
> >         irq = platform_get_irq(pdev, 0);
> > -       if (irq < 0)
> > -               return irq;
> > +       if (irq < 0) {
> > +               err = irq;
> > +               goto fail;

However some reworking is needed in the error path.

That goto statement should be renamed and devm_request_irq() should not
jump to it.

> > +       }
> >
> >         mxic_nfc_hw_init(nfc);
> >
> > --
> > 2.26.2
> >
> >  
> 

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Evgeny Novikov]

Hi Miquel,

On 16.08.2021 11:01, Miquel Raynal wrote:
> Hi Andy,
>
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote on Thu, 12 Aug 2021
> 15:13:10 +0300:
>
>> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru> wrote:
>>
>>> It seems that mxic_nfc_probe() missed invocation of
>>> mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
>>> was refined appropriately.
>>
>> NAK. Until you provide a deeper analysis, like how this works before your
>> change.
>>
>>
>> Please, don’t blindly generate patches, this can even your bot do, just
>> think about each change and preferable test on the real hardware.
>>
>> The above is to all your lovely contributions.
>>
>>
>>> Found by Linux Driver Verification project (linuxtesting.org).
>>>
>>> Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
>>> Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
>>> Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
>>> ---
>>>   drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_
>>> nand.c
>>> index da1070993994..37e75bf60ee5 100644
>>> --- a/drivers/mtd/nand/raw/mxic_nand.c
>>> +++ b/drivers/mtd/nand/raw/mxic_nand.c
>>> @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device
>>> *pdev)
>>>          if (IS_ERR(nfc->send_dly_clk))
>>>                  return PTR_ERR(nfc->send_dly_clk);
>>>
>>> +       err = mxic_nfc_clk_enable(nfc);
>>> +       if (err)
>>> +               return err;
> As Andy said, this is not needed.
>
>>> +
>>>          nfc->regs = devm_platform_ioremap_resource(pdev, 0);
>>> -       if (IS_ERR(nfc->regs))
>>> -               return PTR_ERR(nfc->regs);
>>> +       if (IS_ERR(nfc->regs)) {
>>> +               err = PTR_ERR(nfc->regs);
>>> +               goto fail;
>>> +       }
>>>
>>>          nand_chip = &nfc->chip;
>>>          mtd = nand_to_mtd(nand_chip);
>>> @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device
>>> *pdev)
>>>          nand_chip->controller = &nfc->controller;
>>>
>>>          irq = platform_get_irq(pdev, 0);
>>> -       if (irq < 0)
>>> -               return irq;
>>> +       if (irq < 0) {
>>> +               err = irq;
>>> +               goto fail;
> However some reworking is needed in the error path.
>
> That goto statement should be renamed and devm_request_irq() should not
> jump to it.
>

We still need some help and clarification from those who are very 
familiar with this sort of drivers or/and can test this particular 
driver. mxic_nfc_clk_enable() is the complementary function for 
mxic_nfc_clk_disable(). No other functions invoke 
clk_prepare_enable()/clk_disable_unprepare() in the driver. Unlikely 
somebody in its environment does that since driver specific clocks are 
dealt with. At the moment the driver invokes mxic_nfc_clk_disable() on 
error handling paths in probe, in remove and in mxic_nfc_set_freq(). 
mxic_nfc_clk_enable() is called just by mxic_nfc_set_freq() that 
moreover does this after calling mxic_nfc_clk_disable() first. So, we 
did not find any place in the driver that invokes mxic_nfc_clk_enable() 
prior to mxic_nfc_clk_disable(). Basing on this we added 
mxic_nfc_clk_enable() just after getting clocks. As I explained in the 
previous large e-mail, we may be wrong in our understanding of the 
driver environment or/and at specification of requirements being 
checked. It would be great if you will point out on our mistakes.

Best regards,
Evgeny Novikov
>>> +       }
>>>
>>>          mxic_nfc_hw_init(nfc);
>>>
>>> --
>>> 2.26.2
>>>
>>>   
> Thanks,
> Miquèl

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Miquel Raynal]

Hi Evgeny,

+Mason from Macronix

Evgeny Novikov <novikov@ispras.ru> wrote on Tue, 17 Aug 2021 13:36:03
+0300:

> Hi Miquel,
> 
> On 16.08.2021 11:01, Miquel Raynal wrote:
> > Hi Andy,
> >
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote on Thu, 12 Aug 2021
> > 15:13:10 +0300:
> >  
> >> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru> wrote:
> >>  
> >>> It seems that mxic_nfc_probe() missed invocation of
> >>> mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
> >>> was refined appropriately.  
> >>
> >> NAK. Until you provide a deeper analysis, like how this works before your
> >> change.
> >>
> >>
> >> Please, don’t blindly generate patches, this can even your bot do, just
> >> think about each change and preferable test on the real hardware.
> >>
> >> The above is to all your lovely contributions.
> >>
> >>  
> >>> Found by Linux Driver Verification project (linuxtesting.org).
> >>>
> >>> Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
> >>> Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> >>> Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> >>> ---
> >>>   drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
> >>>   1 file changed, 12 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_
> >>> nand.c
> >>> index da1070993994..37e75bf60ee5 100644
> >>> --- a/drivers/mtd/nand/raw/mxic_nand.c
> >>> +++ b/drivers/mtd/nand/raw/mxic_nand.c
> >>> @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device
> >>> *pdev)
> >>>          if (IS_ERR(nfc->send_dly_clk))
> >>>                  return PTR_ERR(nfc->send_dly_clk);
> >>>
> >>> +       err = mxic_nfc_clk_enable(nfc);
> >>> +       if (err)
> >>> +               return err;  
> > As Andy said, this is not needed.
> >  
> >>> +
> >>>          nfc->regs = devm_platform_ioremap_resource(pdev, 0);
> >>> -       if (IS_ERR(nfc->regs))
> >>> -               return PTR_ERR(nfc->regs);
> >>> +       if (IS_ERR(nfc->regs)) {
> >>> +               err = PTR_ERR(nfc->regs);
> >>> +               goto fail;
> >>> +       }
> >>>
> >>>          nand_chip = &nfc->chip;
> >>>          mtd = nand_to_mtd(nand_chip);
> >>> @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device
> >>> *pdev)
> >>>          nand_chip->controller = &nfc->controller;
> >>>
> >>>          irq = platform_get_irq(pdev, 0);
> >>> -       if (irq < 0)
> >>> -               return irq;
> >>> +       if (irq < 0) {
> >>> +               err = irq;
> >>> +               goto fail;  
> > However some reworking is needed in the error path.
> >
> > That goto statement should be renamed and devm_request_irq() should not
> > jump to it.
> >  
> 
> We still need some help and clarification from those who are very familiar with this sort of drivers or/and can test this particular driver. mxic_nfc_clk_enable() is the complementary function for mxic_nfc_clk_disable(). No other functions invoke clk_prepare_enable()/clk_disable_unprepare() in the driver. Unlikely somebody in its environment does that since driver specific clocks are dealt with. At the moment the driver invokes mxic_nfc_clk_disable() on error handling paths in probe, in remove and in mxic_nfc_set_freq(). mxic_nfc_clk_enable() is called just by mxic_nfc_set_freq() that moreover does this after calling mxic_nfc_clk_disable() first. So, we did not find any place in the driver that invokes mxic_nfc_clk_enable() prior to mxic_nfc_clk_disable(). Basing on this we added mxic_nfc_clk_enable() just after getting clocks. As I explained in the previous large e-mail, we may be wrong in our understanding of the driver environment or/and at specification of requirements being ch
 ecked. It would be great if you will point out on our mistakes.

Enabling the clocks seems to only be needed to access the NAND device
and not the registers of the controller. Mason, is this statement
right? If this statement is wrong, then your proposal is not entirely
wrong in the sense that we must enable the missing clock *before*
accessing any register.

Otherwise for the two other clocks, we don't really care to enable them
before setting the actual frequency in ->setup_interface() (called by
nand_scan()) because at this point we don't yet know what timing mode
is best. Disabling the clock is not an issue even though it was not
enabled in the fist place anyway.

In all cases, the error path is wrong.

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Miquel Raynal]

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Tue, 17 Aug 2021
12:55:21 +0200:

> Hi Evgeny,
> 
> +Mason from Macronix

Mason's e-mail bounced, including YouChing in the discussion who may
also have an answer or perhaps knows who to include in the discussion
(see below for the context and question).

> 
> Evgeny Novikov <novikov@ispras.ru> wrote on Tue, 17 Aug 2021 13:36:03
> +0300:
> 
> > Hi Miquel,
> > 
> > On 16.08.2021 11:01, Miquel Raynal wrote:
> > > Hi Andy,
> > >
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote on Thu, 12 Aug 2021
> > > 15:13:10 +0300:
> > >  
> > >> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru> wrote:
> > >>  
> > >>> It seems that mxic_nfc_probe() missed invocation of
> > >>> mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
> > >>> was refined appropriately.  
> > >>
> > >> NAK. Until you provide a deeper analysis, like how this works before your
> > >> change.
> > >>
> > >>
> > >> Please, don’t blindly generate patches, this can even your bot do, just
> > >> think about each change and preferable test on the real hardware.
> > >>
> > >> The above is to all your lovely contributions.
> > >>
> > >>  
> > >>> Found by Linux Driver Verification project (linuxtesting.org).
> > >>>
> > >>> Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
> > >>> Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> > >>> Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> > >>> ---
> > >>>   drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
> > >>>   1 file changed, 12 insertions(+), 4 deletions(-)
> > >>>
> > >>> diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_
> > >>> nand.c
> > >>> index da1070993994..37e75bf60ee5 100644
> > >>> --- a/drivers/mtd/nand/raw/mxic_nand.c
> > >>> +++ b/drivers/mtd/nand/raw/mxic_nand.c
> > >>> @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device
> > >>> *pdev)
> > >>>          if (IS_ERR(nfc->send_dly_clk))
> > >>>                  return PTR_ERR(nfc->send_dly_clk);
> > >>>
> > >>> +       err = mxic_nfc_clk_enable(nfc);
> > >>> +       if (err)
> > >>> +               return err;  
> > > As Andy said, this is not needed.
> > >  
> > >>> +
> > >>>          nfc->regs = devm_platform_ioremap_resource(pdev, 0);
> > >>> -       if (IS_ERR(nfc->regs))
> > >>> -               return PTR_ERR(nfc->regs);
> > >>> +       if (IS_ERR(nfc->regs)) {
> > >>> +               err = PTR_ERR(nfc->regs);
> > >>> +               goto fail;
> > >>> +       }
> > >>>
> > >>>          nand_chip = &nfc->chip;
> > >>>          mtd = nand_to_mtd(nand_chip);
> > >>> @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device
> > >>> *pdev)
> > >>>          nand_chip->controller = &nfc->controller;
> > >>>
> > >>>          irq = platform_get_irq(pdev, 0);
> > >>> -       if (irq < 0)
> > >>> -               return irq;
> > >>> +       if (irq < 0) {
> > >>> +               err = irq;
> > >>> +               goto fail;  
> > > However some reworking is needed in the error path.
> > >
> > > That goto statement should be renamed and devm_request_irq() should not
> > > jump to it.
> > >  
> > 
> > We still need some help and clarification from those who are very familiar with this sort of drivers or/and can test this particular driver. mxic_nfc_clk_enable() is the complementary function for mxic_nfc_clk_disable(). No other functions invoke clk_prepare_enable()/clk_disable_unprepare() in the driver. Unlikely somebody in its environment does that since driver specific clocks are dealt with. At the moment the driver invokes mxic_nfc_clk_disable() on error handling paths in probe, in remove and in mxic_nfc_set_freq(). mxic_nfc_clk_enable() is called just by mxic_nfc_set_freq() that moreover does this after calling mxic_nfc_clk_disable() first. So, we did not find any place in the driver that invokes mxic_nfc_clk_enable() prior to mxic_nfc_clk_disable(). Basing on this we added mxic_nfc_clk_enable() just after getting clocks. As I explained in the previous large e-mail, we may be wrong in our understanding of the driver environment or/and at specification of requirements being 
 checked. It would be great if you will point out on our mistakes.
> 
> Enabling the clocks seems to only be needed to access the NAND device
> and not the registers of the controller. Mason, is this statement
> right? If this statement is wrong, then your proposal is not entirely
> wrong in the sense that we must enable the missing clock *before*
> accessing any register.
> 
> Otherwise for the two other clocks, we don't really care to enable them
> before setting the actual frequency in ->setup_interface() (called by
> nand_scan()) because at this point we don't yet know what timing mode
> is best. Disabling the clock is not an issue even though it was not
> enabled in the fist place anyway.
> 
> In all cases, the error path is wrong.
> 
> Thanks,
> Miquèl

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[ycllin]

Hi Miquel,

+ Jaime from Macronix.

Jaime will check and confirm this issue.

> Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe
> 
> 
> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Tue, 17 Aug 2021
> 12:55:21 +0200:
> 
> > Hi Evgeny,
> > 
> > +Mason from Macronix
> 
> Mason's e-mail bounced, including YouChing in the discussion who may
> also have an answer or perhaps knows who to include in the discussion
> (see below for the context and question).
> 
> > 
> > Evgeny Novikov <novikov@ispras.ru> wrote on Tue, 17 Aug 2021 13:36:03
> > +0300:
> > 
> > > Hi Miquel,
> > > 
> > > On 16.08.2021 11:01, Miquel Raynal wrote:
> > > > Hi Andy,
> > > >
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote on Thu, 12 Aug 
2021
> > > > 15:13:10 +0300:
> > > > 
> > > >> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru> 
wrote:
> > > >> 
> > > >>> It seems that mxic_nfc_probe() missed invocation of
> > > >>> mxic_nfc_clk_enable(). The patch fixed that. In addition, error 
handling
> > > >>> was refined appropriately. 
> > > >>
> > > >> NAK. Until you provide a deeper analysis, like how this works 
before your
> > > >> change.
> > > >>
> > > >>
> > > >> Please, don’t blindly generate patches, this can even your bot 
do, just
> > > >> think about each change and preferable test on the real hardware.
> > > >>
> > > >> The above is to all your lovely contributions.
> > > >>
> > > >> 
> > > >>> Found by Linux Driver Verification project (linuxtesting.org).
> > > >>>
> > > >>> Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
> > > >>> Co-developed-by: Kirill Shilimanov 
<kirill.shilimanov@huawei.com>
> > > >>> Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> > > >>> ---
> > > >>>   drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
> > > >>>   1 file changed, 12 insertions(+), 4 deletions(-)
> > > >>>
> > > >>> diff --git a/drivers/mtd/nand/raw/mxic_nand.c 
b/drivers/mtd/nand/raw/mxic_
> > > >>> nand.c
> > > >>> index da1070993994..37e75bf60ee5 100644
> > > >>> --- a/drivers/mtd/nand/raw/mxic_nand.c
> > > >>> +++ b/drivers/mtd/nand/raw/mxic_nand.c
> > > >>> @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct 
platform_device
> > > >>> *pdev)
> > > >>>          if (IS_ERR(nfc->send_dly_clk))
> > > >>>                  return PTR_ERR(nfc->send_dly_clk);
> > > >>>
> > > >>> +       err = mxic_nfc_clk_enable(nfc);
> > > >>> +       if (err)
> > > >>> +               return err; 
> > > > As Andy said, this is not needed.
> > > > 
> > > >>> +
> > > >>>          nfc->regs = devm_platform_ioremap_resource(pdev, 0);
> > > >>> -       if (IS_ERR(nfc->regs))
> > > >>> -               return PTR_ERR(nfc->regs);
> > > >>> +       if (IS_ERR(nfc->regs)) {
> > > >>> +               err = PTR_ERR(nfc->regs);
> > > >>> +               goto fail;
> > > >>> +       }
> > > >>>
> > > >>>          nand_chip = &nfc->chip;
> > > >>>          mtd = nand_to_mtd(nand_chip);
> > > >>> @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct 
platform_device
> > > >>> *pdev)
> > > >>>          nand_chip->controller = &nfc->controller;
> > > >>>
> > > >>>          irq = platform_get_irq(pdev, 0);
> > > >>> -       if (irq < 0)
> > > >>> -               return irq;
> > > >>> +       if (irq < 0) {
> > > >>> +               err = irq;
> > > >>> +               goto fail; 
> > > > However some reworking is needed in the error path.
> > > >
> > > > That goto statement should be renamed and devm_request_irq() 
should not
> > > > jump to it.
> > > > 
> > > 
> > > We still need some help and clarification from those who are very 
familiar
> with this sort of drivers or/and can test this particular driver. 
> mxic_nfc_clk_enable() is the complementary function for 
mxic_nfc_clk_disable
> (). No other functions invoke 
clk_prepare_enable()/clk_disable_unprepare() in 
> the driver. Unlikely somebody in its environment does that since driver 
> specific clocks are dealt with. At the moment the driver invokes 
> mxic_nfc_clk_disable() on error handling paths in probe, in remove and 
in 
> mxic_nfc_set_freq(). mxic_nfc_clk_enable() is called just by 
mxic_nfc_set_freq
> () that moreover does this after calling mxic_nfc_clk_disable() first. 
So, we 
> did not find any place in the driver that invokes mxic_nfc_clk_enable() 
prior 
> to mxic_nfc_clk_disable(). Basing on this we added mxic_nfc_clk_enable() 
just 
> after getting clocks. As I explained in the previous large e-mail, we 
may be 
> wrong in our understanding of the driver environment or/and at 
specification 
> of requirements being checked. It would be great if you will point out 
on our mistakes.
> > 
> > Enabling the clocks seems to only be needed to access the NAND device
> > and not the registers of the controller. Mason, is this statement
> > right? If this statement is wrong, then your proposal is not entirely
> > wrong in the sense that we must enable the missing clock *before*
> > accessing any register.
> > 
> > Otherwise for the two other clocks, we don't really care to enable 
them
> > before setting the actual frequency in ->setup_interface() (called by
> > nand_scan()) because at this point we don't yet know what timing mode
> > is best. Disabling the clock is not an issue even though it was not
> > enabled in the fist place anyway.
> > 
> > In all cases, the error path is wrong.
> > 
> > Thanks,
> > Miquèl

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Evgeny Novikov]

Hi Andy,

On 12.08.2021 15:13, Andy Shevchenko wrote:
>
>
> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru 
> <mailto:novikov@ispras.ru>> wrote:
>
>     It seems that mxic_nfc_probe() missed invocation of
>     mxic_nfc_clk_enable(). The patch fixed that. In addition, error
>     handling
>     was refined appropriately.
>
>
> NAK. Until you provide a deeper analysis, like how this works before 
> your change.
>
>
> Please, don’t blindly generate patches, this can even your bot do, 
> just think about each change and preferable test on the real hardware.
>
> The above is to all your lovely contributions.

I completely agree with you that testing and debugging on the real 
hardware is indispensable since this can help to reason about both found 
bugs and patches suggested. Nevertheless, there are several cases when 
this does not work. The most obvious one is lack of appropriate devices 
on the spot that is not an obstacle for static analysis.

My patches are based on results of static verification (software model 
checking). In a nutshell, this approach allows analyzing target programs 
more accurately in comparison with widely used static analysis tools. 
But this is not for free. Usually it needs much more computational 
resources to get something meaningful (in a general case the task is 
undecidable). Modern computer systems solve this issue partially. Worse 
is that thorough static analysis needs more or less complete and correct 
models of environments for target programs. For instance, for loadable 
kernel modules it is necessary to specify correct sequences of callback 
invocations, initialize their arguments at least to some extent and 
develop models of some vital functions like kzalloc(). Moreover, it is 
necessary to specify requirements if one wants to search for something 
special rather than NULL pointer dereferences. We were able to devote a 
relatively small part of our project to development of environment 
models and requirement specifications for verification of the Linux 
kernel. Also, we spent not so much time for application of our framework 
for open source device drivers. Nevertheless, this helped to find out 
quite a lot of bugs many of which are tricky enough. If you are 
interested in more details I can send you our last paper and presentation.

Most our bug reports were accepted by developers. Rarely they preferred 
to fix bugs according to their needs and vision, that is especially the 
case for considerable changes that do need appropriate hardware and 
testing. Just a few our bug reports were ignored or rejected. In the 
latter case developers often pointed out us what is wrong with our 
current understanding and models of the device driver environment or 
requirement specifications. We always welcome this feedback as it allows 
us to refine the stuff and, thus, to avoid false alarms and invalid 
patches. Some developers requested scripts used for finding reported 
issues, but it is not easy to add or refer them in patches like, say, 
for Coccinelle since there is a bunch of external files developed in 
different domain-specific languages as well as a huge verification 
framework, that nobody will include into the source tree of the Linux 
kernel.

Regarding your claim. We do not have an appropriate hardware. As usual 
this bug report was prepared on the base of static verification results 
purely. If you want to see on a particular artifact I based my decision 
on, I can share a link to a visualized violation witness provided by a 
verification tool. We have not any bots since used verification tools do 
not give any suggestions on the issue roots. Maybe in some cases it is 
possible to generate patches automatically on the base of these 
verification results like, say, Coccinelle does, but it is another big 
work. Of course, it is necessary to test the driver and confirm that 
there is an issue or reject the patch. As always the feedback is welcome.

If you are not gratified with my explanation it would be great if you 
and other developers will suggest any ideas how to enhance the process 
if you find this relevant.

Best regards,
Evgeny Novikov

>
>     Found by Linux Driver Verification project (linuxtesting.org
>     <http://linuxtesting.org>).
>
>     Signed-off-by: Evgeny Novikov <novikov@ispras.ru
>     <mailto:novikov@ispras.ru>>
>     Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com
>     <mailto:kirill.shilimanov@huawei.com>>
>     Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com
>     <mailto:kirill.shilimanov@huawei.com>>
>     ---
>      drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
>      1 file changed, 12 insertions(+), 4 deletions(-)
>
>     diff --git a/drivers/mtd/nand/raw/mxic_nand.c
>     b/drivers/mtd/nand/raw/mxic_nand.c
>     index da1070993994..37e75bf60ee5 100644
>     --- a/drivers/mtd/nand/raw/mxic_nand.c
>     +++ b/drivers/mtd/nand/raw/mxic_nand.c
>     @@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct
>     platform_device *pdev)
>             if (IS_ERR(nfc->send_dly_clk))
>                     return PTR_ERR(nfc->send_dly_clk);
>
>     +       err = mxic_nfc_clk_enable(nfc);
>     +       if (err)
>     +               return err;
>     +
>             nfc->regs = devm_platform_ioremap_resource(pdev, 0);
>     -       if (IS_ERR(nfc->regs))
>     -               return PTR_ERR(nfc->regs);
>     +       if (IS_ERR(nfc->regs)) {
>     +               err = PTR_ERR(nfc->regs);
>     +               goto fail;
>     +       }
>
>             nand_chip = &nfc->chip;
>             mtd = nand_to_mtd(nand_chip);
>     @@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct
>     platform_device *pdev)
>             nand_chip->controller = &nfc->controller;
>
>             irq = platform_get_irq(pdev, 0);
>     -       if (irq < 0)
>     -               return irq;
>     +       if (irq < 0) {
>     +               err = irq;
>     +               goto fail;
>     +       }
>
>             mxic_nfc_hw_init(nfc);
>
>     -- 
>     2.26.2
>
>
>
> -- 
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Andy Shevchenko]

On Tue, Aug 17, 2021 at 12:08 PM Evgeny Novikov <novikov@ispras.ru> wrote:
> On 12.08.2021 15:13, Andy Shevchenko wrote:
> > On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru
> > <mailto:novikov@ispras.ru>> wrote:
> >
> >     It seems that mxic_nfc_probe() missed invocation of
> >     mxic_nfc_clk_enable(). The patch fixed that. In addition, error
> >     handling
> >     was refined appropriately.
> >
> > NAK. Until you provide a deeper analysis, like how this works before
> > your change.
> >
> > Please, don’t blindly generate patches, this can even your bot do,
> > just think about each change and preferable test on the real hardware.
> >
> > The above is to all your lovely contributions.
>
> I completely agree with you that testing and debugging on the real
> hardware is indispensable since this can help to reason about both found
> bugs and patches suggested. Nevertheless, there are several cases when
> this does not work. The most obvious one is lack of appropriate devices
> on the spot that is not an obstacle for static analysis.
>
> My patches are based on results of static verification (software model
> checking). In a nutshell, this approach allows analyzing target programs
> more accurately in comparison with widely used static analysis tools.
> But this is not for free. Usually it needs much more computational
> resources to get something meaningful (in a general case the task is
> undecidable). Modern computer systems solve this issue partially. Worse
> is that thorough static analysis needs more or less complete and correct
> models of environments for target programs. For instance, for loadable
> kernel modules it is necessary to specify correct sequences of callback
> invocations, initialize their arguments at least to some extent and
> develop models of some vital functions like kzalloc(). Moreover, it is
> necessary to specify requirements if one wants to search for something
> special rather than NULL pointer dereferences. We were able to devote a
> relatively small part of our project to development of environment
> models and requirement specifications for verification of the Linux
> kernel. Also, we spent not so much time for application of our framework
> for open source device drivers. Nevertheless, this helped to find out
> quite a lot of bugs many of which are tricky enough. If you are
> interested in more details I can send you our last paper and presentation.

It is good and thanks for your contribution!

> Most our bug reports were accepted by developers. Rarely they preferred
> to fix bugs according to their needs and vision, that is especially the
> case for considerable changes that do need appropriate hardware and
> testing. Just a few our bug reports were ignored or rejected.

This ratio is not a point. I hope you learnt from the UMN case.

> In the
> latter case developers often pointed out us what is wrong with our
> current understanding and models of the device driver environment or
> requirement specifications. We always welcome this feedback as it allows
> us to refine the stuff and, thus, to avoid false alarms and invalid
> patches. Some developers requested scripts used for finding reported
> issues, but it is not easy to add or refer them in patches like, say,
> for Coccinelle since there is a bunch of external files developed in
> different domain-specific languages as well as a huge verification
> framework, that nobody will include into the source tree of the Linux
> kernel.
>
> Regarding your claim. We do not have an appropriate hardware. As usual
> this bug report was prepared on the base of static verification results
> purely. If you want to see on a particular artifact I based my decision
> on, I can share a link to a visualized violation witness provided by a
> verification tool. We have not any bots since used verification tools do
> not give any suggestions on the issue roots. Maybe in some cases it is
> possible to generate patches automatically on the base of these
> verification results like, say, Coccinelle does, but it is another big
> work. Of course, it is necessary to test the driver and confirm that
> there is an issue or reject the patch. As always the feedback is welcome.

My point is that the type of patches you are sending even a bot may
generate (for example, simple patches the LKP project generates along
with reports). The problem with all teams that are working with static
analysers against Linux kernel is that they so proud of their tools
and trying to flood the mailing lists with quick and nice fixes, from
which some are churn, some are simple bad, some are _bringing_
regressions, and only some are good enough. The ratio seems to me like
1 to 4 at most. So, 75% patches are not needed and only are a burden
on maintainers' shoulders.

Good patch should have a good commit message [1]. The message should
include an analysis to explain why the considered change is needed and
what the problem it tries to solve. Neither of this I have seen in
your patch. Also, you need to take into account the credits and tags
that Linux kernel is using (Fixes, Suggested-by, Reported-by, etc) it
will add a bit of unification. Also, while fixing problems these
patches often miss the big picture, and contributors should think
outside the box (this is a problem of 95% of such contributions, one
example is your patch where I recommended completely rewriting the
->probe() approach). That said, I don't want to see the flood of
patches with 75% miss ratio, I want to see maybe 5x, 10x less patches,
but each of them is carefully thought through and _ideally_ be tested
besides compilation.

And thank you for your work!

> If you are not gratified with my explanation it would be great if you
> and other developers will suggest any ideas how to enhance the process
> if you find this relevant.

[1]: https://chris.beams.io/posts/git-commit/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Evgeny Novikov]

Hi Andy,

On 17.08.2021 14:47, Andy Shevchenko wrote:
> On Tue, Aug 17, 2021 at 12:08 PM Evgeny Novikov <novikov@ispras.ru> wrote:
>> On 12.08.2021 15:13, Andy Shevchenko wrote:
>>> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru
>>> <mailto:novikov@ispras.ru>> wrote:
>>>
>>>      It seems that mxic_nfc_probe() missed invocation of
>>>      mxic_nfc_clk_enable(). The patch fixed that. In addition, error
>>>      handling
>>>      was refined appropriately.
>>>
>>> NAK. Until you provide a deeper analysis, like how this works before
>>> your change.
>>>
>>> Please, don’t blindly generate patches, this can even your bot do,
>>> just think about each change and preferable test on the real hardware.
>>>
>>> The above is to all your lovely contributions.
>> I completely agree with you that testing and debugging on the real
>> hardware is indispensable since this can help to reason about both found
>> bugs and patches suggested. Nevertheless, there are several cases when
>> this does not work. The most obvious one is lack of appropriate devices
>> on the spot that is not an obstacle for static analysis.
>>
>> My patches are based on results of static verification (software model
>> checking). In a nutshell, this approach allows analyzing target programs
>> more accurately in comparison with widely used static analysis tools.
>> But this is not for free. Usually it needs much more computational
>> resources to get something meaningful (in a general case the task is
>> undecidable). Modern computer systems solve this issue partially. Worse
>> is that thorough static analysis needs more or less complete and correct
>> models of environments for target programs. For instance, for loadable
>> kernel modules it is necessary to specify correct sequences of callback
>> invocations, initialize their arguments at least to some extent and
>> develop models of some vital functions like kzalloc(). Moreover, it is
>> necessary to specify requirements if one wants to search for something
>> special rather than NULL pointer dereferences. We were able to devote a
>> relatively small part of our project to development of environment
>> models and requirement specifications for verification of the Linux
>> kernel. Also, we spent not so much time for application of our framework
>> for open source device drivers. Nevertheless, this helped to find out
>> quite a lot of bugs many of which are tricky enough. If you are
>> interested in more details I can send you our last paper and presentation.
> It is good and thanks for your contribution!
>
>> Most our bug reports were accepted by developers. Rarely they preferred
>> to fix bugs according to their needs and vision, that is especially the
>> case for considerable changes that do need appropriate hardware and
>> testing. Just a few our bug reports were ignored or rejected.
> This ratio is not a point. I hope you learnt from the UMN case.
>
>> In the
>> latter case developers often pointed out us what is wrong with our
>> current understanding and models of the device driver environment or
>> requirement specifications. We always welcome this feedback as it allows
>> us to refine the stuff and, thus, to avoid false alarms and invalid
>> patches. Some developers requested scripts used for finding reported
>> issues, but it is not easy to add or refer them in patches like, say,
>> for Coccinelle since there is a bunch of external files developed in
>> different domain-specific languages as well as a huge verification
>> framework, that nobody will include into the source tree of the Linux
>> kernel.
>>
>> Regarding your claim. We do not have an appropriate hardware. As usual
>> this bug report was prepared on the base of static verification results
>> purely. If you want to see on a particular artifact I based my decision
>> on, I can share a link to a visualized violation witness provided by a
>> verification tool. We have not any bots since used verification tools do
>> not give any suggestions on the issue roots. Maybe in some cases it is
>> possible to generate patches automatically on the base of these
>> verification results like, say, Coccinelle does, but it is another big
>> work. Of course, it is necessary to test the driver and confirm that
>> there is an issue or reject the patch. As always the feedback is welcome.
> My point is that the type of patches you are sending even a bot may
> generate (for example, simple patches the LKP project generates along
> with reports). The problem with all teams that are working with static
> analysers against Linux kernel is that they so proud of their tools
> and trying to flood the mailing lists with quick and nice fixes, from
> which some are churn, some are simple bad, some are _bringing_
> regressions, and only some are good enough. The ratio seems to me like
> 1 to 4 at most. So, 75% patches are not needed and only are a burden
> on maintainers' shoulders.
Developers of static analysis tools need some acknowledgment. 
Application to the Linux kernel gives a great capability for that since 
it is a huge and vital open source project. Besides, it is unlikely that 
somebody will be able to develop any valuable QA tool without a numerous 
feedback from users (in case of this sort of tools users are developers 
of target projects). We always welcome any ideas and suggestions how to 
improve a quality of analysis.
> Good patch should have a good commit message [1]. The message should
> include an analysis to explain why the considered change is needed and
> what the problem it tries to solve. Neither of this I have seen in
> your patch. Also, you need to take into account the credits and tags
> that Linux kernel is using (Fixes, Suggested-by, Reported-by, etc) it
> will add a bit of unification. Also, while fixing problems these
> patches often miss the big picture, and contributors should think
> outside the box (this is a problem of 95% of such contributions, one
> example is your patch where I recommended completely rewriting the
> ->probe() approach). That said, I don't want to see the flood of
> patches with 75% miss ratio, I want to see maybe 5x, 10x less patches,
> but each of them is carefully thought through and _ideally_ be tested
> besides compilation.
We will try to follow your advices to a possible extent. I am not sure 
that this will be the case for thinking outside the box since often it 
requires a deep involvement into the development process. Moreover, it 
may be dangerous to make such big changes without having an appropriate 
experience or/and an ability to test them.
> And thank you for your work!
Thank you for your patience!

Best regards,
Evgeny Novikov
>> If you are not gratified with my explanation it would be great if you
>> and other developers will suggest any ideas how to enhance the process
>> if you find this relevant.
> [1]: https://chris.beams.io/posts/git-commit/
>

Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

[Andy Shevchenko]

On Sat, Aug 21, 2021 at 8:26 PM Evgeny Novikov <novikov@ispras.ru> wrote:
> On 17.08.2021 14:47, Andy Shevchenko wrote:
> > On Tue, Aug 17, 2021 at 12:08 PM Evgeny Novikov <novikov@ispras.ru> wrote:
> >> On 12.08.2021 15:13, Andy Shevchenko wrote:
> >>> On Thursday, August 12, 2021, Evgeny Novikov <novikov@ispras.ru
> >>> <mailto:novikov@ispras.ru>> wrote:
> >>>
> >>>      It seems that mxic_nfc_probe() missed invocation of
> >>>      mxic_nfc_clk_enable(). The patch fixed that. In addition, error
> >>>      handling
> >>>      was refined appropriately.
> >>>
> >>> NAK. Until you provide a deeper analysis, like how this works before
> >>> your change.
> >>>
> >>> Please, don’t blindly generate patches, this can even your bot do,
> >>> just think about each change and preferable test on the real hardware.
> >>>
> >>> The above is to all your lovely contributions.
> >> I completely agree with you that testing and debugging on the real
> >> hardware is indispensable since this can help to reason about both found
> >> bugs and patches suggested. Nevertheless, there are several cases when
> >> this does not work. The most obvious one is lack of appropriate devices
> >> on the spot that is not an obstacle for static analysis.
> >>
> >> My patches are based on results of static verification (software model
> >> checking). In a nutshell, this approach allows analyzing target programs
> >> more accurately in comparison with widely used static analysis tools.
> >> But this is not for free. Usually it needs much more computational
> >> resources to get something meaningful (in a general case the task is
> >> undecidable). Modern computer systems solve this issue partially. Worse
> >> is that thorough static analysis needs more or less complete and correct
> >> models of environments for target programs. For instance, for loadable
> >> kernel modules it is necessary to specify correct sequences of callback
> >> invocations, initialize their arguments at least to some extent and
> >> develop models of some vital functions like kzalloc(). Moreover, it is
> >> necessary to specify requirements if one wants to search for something
> >> special rather than NULL pointer dereferences. We were able to devote a
> >> relatively small part of our project to development of environment
> >> models and requirement specifications for verification of the Linux
> >> kernel. Also, we spent not so much time for application of our framework
> >> for open source device drivers. Nevertheless, this helped to find out
> >> quite a lot of bugs many of which are tricky enough. If you are
> >> interested in more details I can send you our last paper and presentation.
> > It is good and thanks for your contribution!
> >
> >> Most our bug reports were accepted by developers. Rarely they preferred
> >> to fix bugs according to their needs and vision, that is especially the
> >> case for considerable changes that do need appropriate hardware and
> >> testing. Just a few our bug reports were ignored or rejected.
> > This ratio is not a point. I hope you learnt from the UMN case.
> >
> >> In the
> >> latter case developers often pointed out us what is wrong with our
> >> current understanding and models of the device driver environment or
> >> requirement specifications. We always welcome this feedback as it allows
> >> us to refine the stuff and, thus, to avoid false alarms and invalid
> >> patches. Some developers requested scripts used for finding reported
> >> issues, but it is not easy to add or refer them in patches like, say,
> >> for Coccinelle since there is a bunch of external files developed in
> >> different domain-specific languages as well as a huge verification
> >> framework, that nobody will include into the source tree of the Linux
> >> kernel.
> >>
> >> Regarding your claim. We do not have an appropriate hardware. As usual
> >> this bug report was prepared on the base of static verification results
> >> purely. If you want to see on a particular artifact I based my decision
> >> on, I can share a link to a visualized violation witness provided by a
> >> verification tool. We have not any bots since used verification tools do
> >> not give any suggestions on the issue roots. Maybe in some cases it is
> >> possible to generate patches automatically on the base of these
> >> verification results like, say, Coccinelle does, but it is another big
> >> work. Of course, it is necessary to test the driver and confirm that
> >> there is an issue or reject the patch. As always the feedback is welcome.
> > My point is that the type of patches you are sending even a bot may
> > generate (for example, simple patches the LKP project generates along
> > with reports). The problem with all teams that are working with static
> > analysers against Linux kernel is that they so proud of their tools
> > and trying to flood the mailing lists with quick and nice fixes, from
> > which some are churn, some are simple bad, some are _bringing_
> > regressions, and only some are good enough. The ratio seems to me like
> > 1 to 4 at most. So, 75% patches are not needed and only are a burden
> > on maintainers' shoulders.
> Developers of static analysis tools need some acknowledgment.
> Application to the Linux kernel gives a great capability for that since
> it is a huge and vital open source project. Besides, it is unlikely that
> somebody will be able to develop any valuable QA tool without a numerous
> feedback from users (in case of this sort of tools users are developers
> of target projects). We always welcome any ideas and suggestions how to
> improve a quality of analysis.

Good luck with it!

> > Good patch should have a good commit message [1]. The message should
> > include an analysis to explain why the considered change is needed and
> > what the problem it tries to solve. Neither of this I have seen in
> > your patch. Also, you need to take into account the credits and tags
> > that Linux kernel is using (Fixes, Suggested-by, Reported-by, etc) it
> > will add a bit of unification. Also, while fixing problems these
> > patches often miss the big picture, and contributors should think
> > outside the box (this is a problem of 95% of such contributions, one
> > example is your patch where I recommended completely rewriting the
> > ->probe() approach). That said, I don't want to see the flood of
> > patches with 75% miss ratio, I want to see maybe 5x, 10x less patches,
> > but each of them is carefully thought through and _ideally_ be tested
> > besides compilation.
> We will try to follow your advices to a possible extent. I am not sure
> that this will be the case for thinking outside the box since often it
> requires a deep involvement into the development process.

Exactly my point. You need to dive into development better.

> Moreover, it
> may be dangerous to make such big changes without having an appropriate
> experience or/and an ability to test them.
> > And thank you for your work!
> Thank you for your patience!

> >> If you are not gratified with my explanation it would be great if you
> >> and other developers will suggest any ideas how to enhance the process
> >> if you find this relevant.
> > [1]: https://chris.beams.io/posts/git-commit/

-- 
With Best Regards,
Andy Shevchenko