| Message ID | 20230123012940.1250879-1-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: iommu: renesas,ipmmu-vmsa: Update descriptions for R-Car Gen4 | expand |
On 23/01/2023 02:29, Yoshihiro Shimoda wrote: > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > each cache IPMMU has own module id. So, update descriptions of > renesas,ipmmu-main property for R-Car Gen4. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > the latest datasheet undocumented the register. So, update the propeties > description. Note that the second argument is not used on the driver. > So no behavior change. > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Hi Shimoda-san, On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > each cache IPMMU has own module id. So, update descriptions of > renesas,ipmmu-main property for R-Car Gen4. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > the latest datasheet undocumented the register. So, update the propeties > description. Note that the second argument is not used on the driver. DT describes hardware, not software policy. > So no behavior change. So where do we get the module id numbers to use, if they are no longer documented in the Hardware Manual? > --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > @@ -76,14 +76,15 @@ properties: > items: > - items: > - description: phandle to main IPMMU > - - description: the interrupt bit number associated with the particular > - cache IPMMU device. The interrupt bit number needs to match the main > - IPMMU IMSSTR register. Only used by cache IPMMU instances. > + - description: The interrupt bit number or module id associated with > + the particular cache IPMMU device. The interrupt bit number needs > + to match the main IPMMU IMSSTR register. Only used by cache IPMMU > + instances. > description: > Reference to the main IPMMU phandle plus 1 cell. The cell is > - the interrupt bit number associated with the particular cache IPMMU > - device. The interrupt bit number needs to match the main IPMMU IMSSTR > - register. Only used by cache IPMMU instances. > + the interrupt bit number or module id associated with the particular > + cache IPMMU device. The interrupt bit number needs to match the main > + IPMMU IMSSTR register. Only used by cache IPMMU instances. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM > > Hi Shimoda-san, > > On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > > each cache IPMMU has own module id. So, update descriptions of > > renesas,ipmmu-main property for R-Car Gen4. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! Thank you for your review! > > --- > > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > > the latest datasheet undocumented the register. So, update the propeties > > description. Note that the second argument is not used on the driver. > > DT describes hardware, not software policy. I think so. > > So no behavior change. > > So where do we get the module id numbers to use, if they are no longer > documented in the Hardware Manual? If so, we cannot get the module id numbers. So, should we use other information which is completely fixed instead? I have some ideas: 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. 2) Sequential numbers from register base offset. In R-Car S4: ipmmu_rt0 is the first node from register base offset, and ipmmu_rt1 is the second one. So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. 3) Using base address upper 16-bits. In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. Perhaps, the option 1) is reasonable, I think. But, what do you think? Best regards, Yoshihiro Shimoda > > --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > > @@ -76,14 +76,15 @@ properties: > > items: > > - items: > > - description: phandle to main IPMMU > > - - description: the interrupt bit number associated with the particular > > - cache IPMMU device. The interrupt bit number needs to match the main > > - IPMMU IMSSTR register. Only used by cache IPMMU instances. > > + - description: The interrupt bit number or module id associated with > > + the particular cache IPMMU device. The interrupt bit number needs > > + to match the main IPMMU IMSSTR register. Only used by cache IPMMU > > + instances. > > description: > > Reference to the main IPMMU phandle plus 1 cell. The cell is > > - the interrupt bit number associated with the particular cache IPMMU > > - device. The interrupt bit number needs to match the main IPMMU IMSSTR > > - register. Only used by cache IPMMU instances. > > + the interrupt bit number or module id associated with the particular > > + cache IPMMU device. The interrupt bit number needs to match the main > > + IPMMU IMSSTR register. Only used by cache IPMMU instances. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Shimoda-san, On Wed, Jan 25, 2023 at 1:49 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM > > On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > > > each cache IPMMU has own module id. So, update descriptions of > > > renesas,ipmmu-main property for R-Car Gen4. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > --- > > > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > > > the latest datasheet undocumented the register. So, update the propeties > > > description. Note that the second argument is not used on the driver. > > > > DT describes hardware, not software policy. > > I think so. > > > > So no behavior change. > > > > So where do we get the module id numbers to use, if they are no longer > > documented in the Hardware Manual? > > If so, we cannot get the module id numbers. So, should we use other > information which is completely fixed instead? I have some ideas: > 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. > 2) Sequential numbers from register base offset. > In R-Car S4: ipmmu_rt0 is the first node from register base offset, > and ipmmu_rt1 is the second one. > So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. > 3) Using base address upper 16-bits. > In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. > > Perhaps, the option 1) is reasonable, I think. But, what do you think? I would not make up numbers, as that would cause confusion with SoCs where the numbers do match the hardware. As the driver doesn't use the module id number (it already loops over all domains, instead of checking IMSSTR, probably because of historical (R-Car Gen2) reasons?), what about dropping it from the property? I.e. add "minItems: 1", possibly only when compatible with renesas,rcar-gen4-ipmmu-vmsa? BTW, the related IMCTR register is still documented, and the driver does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the hardware (documentation) people intend this to be used... Perhaps IMCTR_INTEN will be removed/undocumented, too? Or perhaps the removal/undocumentation of IMSSTR was a mistake? > > > --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > > > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml > > > @@ -76,14 +76,15 @@ properties: > > > items: > > > - items: > > > - description: phandle to main IPMMU > > > - - description: the interrupt bit number associated with the particular > > > - cache IPMMU device. The interrupt bit number needs to match the main > > > - IPMMU IMSSTR register. Only used by cache IPMMU instances. > > > + - description: The interrupt bit number or module id associated with > > > + the particular cache IPMMU device. The interrupt bit number needs > > > + to match the main IPMMU IMSSTR register. Only used by cache IPMMU > > > + instances. > > > description: > > > Reference to the main IPMMU phandle plus 1 cell. The cell is > > > - the interrupt bit number associated with the particular cache IPMMU > > > - device. The interrupt bit number needs to match the main IPMMU IMSSTR > > > - register. Only used by cache IPMMU instances. > > > + the interrupt bit number or module id associated with the particular > > > + cache IPMMU device. The interrupt bit number needs to match the main > > > + IPMMU IMSSTR register. Only used by cache IPMMU instances. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 2023-01-25 08:54, Geert Uytterhoeven wrote: > Hi Shimoda-san, > > On Wed, Jan 25, 2023 at 1:49 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: >>> From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM >>> On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda >>> <yoshihiro.shimoda.uh@renesas.com> wrote: >>>> Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but >>>> each cache IPMMU has own module id. So, update descriptions of >>>> renesas,ipmmu-main property for R-Car Gen4. >>>> >>>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > >>>> --- >>>> The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but >>>> the latest datasheet undocumented the register. So, update the propeties >>>> description. Note that the second argument is not used on the driver. >>> >>> DT describes hardware, not software policy. >> >> I think so. >> >>>> So no behavior change. >>> >>> So where do we get the module id numbers to use, if they are no longer >>> documented in the Hardware Manual? >> >> If so, we cannot get the module id numbers. So, should we use other >> information which is completely fixed instead? I have some ideas: >> 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. >> 2) Sequential numbers from register base offset. >> In R-Car S4: ipmmu_rt0 is the first node from register base offset, >> and ipmmu_rt1 is the second one. >> So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. >> 3) Using base address upper 16-bits. >> In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. >> >> Perhaps, the option 1) is reasonable, I think. But, what do you think? > > I would not make up numbers, as that would cause confusion with SoCs > where the numbers do match the hardware. > As the driver doesn't use the module id number (it already loops > over all domains, instead of checking IMSSTR, probably because of > historical (R-Car Gen2) reasons?), what about dropping it from the > property? I.e. add "minItems: 1", possibly only when compatible with > renesas,rcar-gen4-ipmmu-vmsa? Right, if there really is no meaningful ID for this model then its binding should not require one. > BTW, the related IMCTR register is still documented, and the driver > does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the > hardware (documentation) people intend this to be used... > Perhaps IMCTR_INTEN will be removed/undocumented, too? > Or perhaps the removal/undocumentation of IMSSTR was a mistake? I guess it should be pretty straightforward to just try reading the expected IMSSTR register locations on this SoC to double-check whether anything is there. Thanks, Robin.
Hi Robin, On Wed, Jan 25, 2023 at 11:42 AM Robin Murphy <robin.murphy@arm.com> wrote: > On 2023-01-25 08:54, Geert Uytterhoeven wrote: > > BTW, the related IMCTR register is still documented, and the driver > > does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the > > hardware (documentation) people intend this to be used... > > Perhaps IMCTR_INTEN will be removed/undocumented, too? > > Or perhaps the removal/undocumentation of IMSSTR was a mistake? > > I guess it should be pretty straightforward to just try reading the > expected IMSSTR register locations on this SoC to double-check whether > anything is there. FTR, I did try this. Unfortunately the IMSSTR register value is zero by default, and interrupts are generated only for error conditions (how to trigger them?), so my debug print in ipmmu_irq() never triggered. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Shimoda-san, On Thu, Jan 26, 2023 at 1:55 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, January 25, 2023 5:55 PM > > On Wed, Jan 25, 2023 at 1:49 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM > > > > On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > > > > > each cache IPMMU has own module id. So, update descriptions of > > > > > renesas,ipmmu-main property for R-Car Gen4. > > > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > --- > > > > > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > > > > > the latest datasheet undocumented the register. So, update the propeties > > > > > description. Note that the second argument is not used on the driver. > > > > > > > > DT describes hardware, not software policy. > > > > > > I think so. > > > > > > > > So no behavior change. > > > > > > > > So where do we get the module id numbers to use, if they are no longer > > > > documented in the Hardware Manual? > > > > > > If so, we cannot get the module id numbers. So, should we use other > > > information which is completely fixed instead? I have some ideas: > > > 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. > > > 2) Sequential numbers from register base offset. > > > In R-Car S4: ipmmu_rt0 is the first node from register base offset, > > > and ipmmu_rt1 is the second one. > > > So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. > > > 3) Using base address upper 16-bits. > > > In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. > > > > > > Perhaps, the option 1) is reasonable, I think. But, what do you think? > > > > I would not make up numbers, as that would cause confusion with SoCs > > where the numbers do match the hardware. > > I see. > > > As the driver doesn't use the module id number (it already loops > > over all domains, instead of checking IMSSTR, probably because of > > historical (R-Car Gen2) reasons?), what about dropping it from the > > property? I.e. add "minItems: 1", possibly only when compatible with > > renesas,rcar-gen4-ipmmu-vmsa? > > It looks a good idea to me. > > > BTW, the related IMCTR register is still documented, and the driver > > does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the > > hardware (documentation) people intend this to be used... > > Perhaps IMCTR_INTEN will be removed/undocumented, too? > > Or perhaps the removal/undocumentation of IMSSTR was a mistake? > > I don't think that IMCTR_INTEN will be removed/undocumented too because > the IMCTR register is related to IMSTR register, not IMSSTR register ;) > ~~~~~ ~~~~~~ > About undocumentation of IMSSTR, I found that accessing the register is > possible to cause a potential issue on both R-Car Gen3/4. That's why > the IMSSTR register is undocumented on R-Car Gen4. I'm not sure whether > R-Car Gen3 will be undocumented too in the future though, but at least, > we should not access the IMSSTR register on both R-Car Gen3/Gen4. > # I'm not sure, but that's why the driver doesn't check IMSSTR to avoid > # the potential issue?? IC > So, to simplify the dt-bindings, just removing the second property without > any condition is better, I think. But, what do you think? So just add "minItems: 1", and leave out the second value of the "renesas,ipmmu-main" property when adding support for new SoCs. I don't think there is an immediate need to remove the existing second value on already supported SoCs, as these values are not incorrect hardware description. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert-san, > From: Geert Uytterhoeven <geert@linux-m68k.org>, Sent: Thursday, January 26, 2023 5:38 PM > > On Thu, Jan 26, 2023 at 1:55 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Wednesday, January 25, 2023 5:55 PM > > > On Wed, Jan 25, 2023 at 1:49 AM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM > > > > > On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda > > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > > Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > > > > > > each cache IPMMU has own module id. So, update descriptions of > > > > > > renesas,ipmmu-main property for R-Car Gen4. > > > > > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > > > --- > > > > > > The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > > > > > > the latest datasheet undocumented the register. So, update the propeties > > > > > > description. Note that the second argument is not used on the driver. > > > > > > > > > > DT describes hardware, not software policy. > > > > > > > > I think so. > > > > > > > > > > So no behavior change. > > > > > > > > > > So where do we get the module id numbers to use, if they are no longer > > > > > documented in the Hardware Manual? > > > > > > > > If so, we cannot get the module id numbers. So, should we use other > > > > information which is completely fixed instead? I have some ideas: > > > > 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. > > > > 2) Sequential numbers from register base offset. > > > > In R-Car S4: ipmmu_rt0 is the first node from register base offset, > > > > and ipmmu_rt1 is the second one. > > > > So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. > > > > 3) Using base address upper 16-bits. > > > > In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. > > > > > > > > Perhaps, the option 1) is reasonable, I think. But, what do you think? > > > > > > I would not make up numbers, as that would cause confusion with SoCs > > > where the numbers do match the hardware. > > > > I see. > > > > > As the driver doesn't use the module id number (it already loops > > > over all domains, instead of checking IMSSTR, probably because of > > > historical (R-Car Gen2) reasons?), what about dropping it from the > > > property? I.e. add "minItems: 1", possibly only when compatible with > > > renesas,rcar-gen4-ipmmu-vmsa? > > > > It looks a good idea to me. > > > > > BTW, the related IMCTR register is still documented, and the driver > > > does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the > > > hardware (documentation) people intend this to be used... > > > Perhaps IMCTR_INTEN will be removed/undocumented, too? > > > Or perhaps the removal/undocumentation of IMSSTR was a mistake? > > > > I don't think that IMCTR_INTEN will be removed/undocumented too because > > the IMCTR register is related to IMSTR register, not IMSSTR register ;) > > ~~~~~ ~~~~~~ > > About undocumentation of IMSSTR, I found that accessing the register is > > possible to cause a potential issue on both R-Car Gen3/4. That's why > > the IMSSTR register is undocumented on R-Car Gen4. I'm not sure whether > > R-Car Gen3 will be undocumented too in the future though, but at least, > > we should not access the IMSSTR register on both R-Car Gen3/Gen4. > > # I'm not sure, but that's why the driver doesn't check IMSSTR to avoid > > # the potential issue?? > > IC > > > So, to simplify the dt-bindings, just removing the second property without > > any condition is better, I think. But, what do you think? > > So just add "minItems: 1", and leave out the second value of the > "renesas,ipmmu-main" property when adding support for new SoCs. > I don't think there is an immediate need to remove the existing second > value on already supported SoCs, as these values are not incorrect > hardware description. Thank you for your comment! I understood it. So, I'll submit such a patch as v2 tomorrow. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml index 72308a4c14e7..7f63ecb467e6 100644 --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml @@ -76,14 +76,15 @@ properties: items: - items: - description: phandle to main IPMMU - - description: the interrupt bit number associated with the particular - cache IPMMU device. The interrupt bit number needs to match the main - IPMMU IMSSTR register. Only used by cache IPMMU instances. + - description: The interrupt bit number or module id associated with + the particular cache IPMMU device. The interrupt bit number needs + to match the main IPMMU IMSSTR register. Only used by cache IPMMU + instances. description: Reference to the main IPMMU phandle plus 1 cell. The cell is - the interrupt bit number associated with the particular cache IPMMU - device. The interrupt bit number needs to match the main IPMMU IMSSTR - register. Only used by cache IPMMU instances. + the interrupt bit number or module id associated with the particular + cache IPMMU device. The interrupt bit number needs to match the main + IPMMU IMSSTR register. Only used by cache IPMMU instances. required: - compatible
Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but each cache IPMMU has own module id. So, update descriptions of renesas,ipmmu-main property for R-Car Gen4. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but the latest datasheet undocumented the register. So, update the propeties description. Note that the second argument is not used on the driver. So no behavior change. .../bindings/iommu/renesas,ipmmu-vmsa.yaml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)