| Message ID | 1442938118-4718-2-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/22/2015 12:08 PM, Nishanth Menon wrote: > Keystone2 devices are used on more platforms than just Texas > Instruments reference evaluation platforms called EVMs. Providing a > generic compatible "ti,keystone" is not sufficient to differentiate > various SoC definitions possible on various platforms. So, provide > compatible matches for each SoC family by itself. > > This allows SoC specific logic to be run time handled based on > of_machine_is_compatible("ti,k2hk") or as needed for the dependent > processor instead of needing to use board dependent compatibles that > are needed now. > > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > .../devicetree/bindings/arm/keystone/keystone.txt | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/keystone/keystone.txt b/Documentation/devicetree/bindings/arm/keystone/keystone.txt > index 59d7a46f85eb..800d2d02e27b 100644 > --- a/Documentation/devicetree/bindings/arm/keystone/keystone.txt > +++ b/Documentation/devicetree/bindings/arm/keystone/keystone.txt > @@ -9,12 +9,26 @@ Required properties: > the form "ti,keystone-*". Generic devices like gic, arch_timers, ns16550 > type UART should use the specified compatible for those devices. > > +SoC families: > + > +- Keystone 2 generic SoC: > + compatible = "ti,keystone" > + > +SoCs: > + > +- Keystone 2 Hawking/Kepler > + compatible = ti,k2hk", "ti,keystone" > +- Keystone 2 Lamarr > + compatible = ti,k2l", "ti,keystone" > +- Keystone 2 Edison > + compatible = ti,k2e", "ti,keystone" > + > Boards: > - Keystone 2 Hawking/Kepler EVM > - compatible = "ti,k2hk-evm","ti,keystone" > + compatible = "ti,k2hk-evm", "ti,k2hk", "ti,keystone" > > - Keystone 2 Lamarr EVM > - compatible = "ti,k2l-evm","ti,keystone" > + compatible = "ti,k2l-evm", "ti, k2l", "ti,keystone" > > - Keystone 2 Edison EVM > - compatible = "ti,k2e-evm","ti,keystone" > + compatible = "ti,k2e-evm", "ti,k2e", "ti,keystone" > DTS takes care of the difference in the hardware and If there SoC specific customization required outside this, then it is best to include this as part of that change. In the past, I believe we didn't do it due to the same reason as above. Murali
Nishant, On 9/22/2015 9:08 AM, Nishanth Menon wrote: > Keystone2 devices are used on more platforms than just Texas > Instruments reference evaluation platforms called EVMs. Providing a > generic compatible "ti,keystone" is not sufficient to differentiate > various SoC definitions possible on various platforms. So, provide > compatible matches for each SoC family by itself. > > This allows SoC specific logic to be run time handled based on > of_machine_is_compatible("ti,k2hk") or as needed for the dependent > processor instead of needing to use board dependent compatibles that > are needed now. > > Signed-off-by: Nishanth Menon <nm@ti.com> > --- You need to expand that 'not sufficient' for me. Unless there is genuine case to support this, I would want to avoid this churn. Regards, Santosh
On 09/23/2015 01:05 PM, Murali Karicheri wrote: > On 09/22/2015 12:08 PM, Nishanth Menon wrote: [...] >> diff --git >> a/Documentation/devicetree/bindings/arm/keystone/keystone.txt >> b/Documentation/devicetree/bindings/arm/keystone/keystone.txt >> index 59d7a46f85eb..800d2d02e27b 100644 >> --- a/Documentation/devicetree/bindings/arm/keystone/keystone.txt >> +++ b/Documentation/devicetree/bindings/arm/keystone/keystone.txt >> @@ -9,12 +9,26 @@ Required properties: >> the form "ti,keystone-*". Generic devices like gic, arch_timers, >> ns16550 >> type UART should use the specified compatible for those devices. >> >> +SoC families: >> + >> +- Keystone 2 generic SoC: >> + compatible = "ti,keystone" >> + >> +SoCs: >> + >> +- Keystone 2 Hawking/Kepler >> + compatible = ti,k2hk", "ti,keystone" >> +- Keystone 2 Lamarr >> + compatible = ti,k2l", "ti,keystone" >> +- Keystone 2 Edison >> + compatible = ti,k2e", "ti,keystone" >> + >> Boards: >> - Keystone 2 Hawking/Kepler EVM >> - compatible = "ti,k2hk-evm","ti,keystone" >> + compatible = "ti,k2hk-evm", "ti,k2hk", "ti,keystone" >> >> - Keystone 2 Lamarr EVM >> - compatible = "ti,k2l-evm","ti,keystone" >> + compatible = "ti,k2l-evm", "ti, k2l", "ti,keystone" >> >> - Keystone 2 Edison EVM >> - compatible = "ti,k2e-evm","ti,keystone" >> + compatible = "ti,k2e-evm", "ti,k2e", "ti,keystone" >> > DTS takes care of the difference in the hardware and If there SoC > specific customization required outside this, then it is best to include > this as part of that change. In the past, I believe we didn't do it due > to the same reason as above. Right now, they all claim to be the same keystone SoC -> which is inaccurate at the very least, and downright wrong IMHO since k2e/hk and l are different SoCs - even though they belong to keystone family, they are as similar as OMAP4 is to OMAP5 is to DRA7. This is similar to how various other SoCs distinguish between SoCs as well.
On 09/23/2015 02:19 PM, santosh shilimkar wrote: > Nishant, > > On 9/22/2015 9:08 AM, Nishanth Menon wrote: >> Keystone2 devices are used on more platforms than just Texas >> Instruments reference evaluation platforms called EVMs. Providing a >> generic compatible "ti,keystone" is not sufficient to differentiate >> various SoC definitions possible on various platforms. So, provide >> compatible matches for each SoC family by itself. >> >> This allows SoC specific logic to be run time handled based on >> of_machine_is_compatible("ti,k2hk") or as needed for the dependent >> processor instead of needing to use board dependent compatibles that >> are needed now. >> >> Signed-off-by: Nishanth Menon <nm@ti.com> >> --- > You need to expand that 'not sufficient' for me. Unless there is > genuine case to support this, I would want to avoid this churn. > I agree. If there are run time check required in code to treat the variants of Keystone SoC differently, then this change is needed. At this time, SoC DTS captures these differences. IMHO, If a future Keystone SoC support required SoC specific compatibility string to customize SoC specific initialization code, then it can be introduced at that time. I am not sure why this is introduced with out an example usage. > Regards, > Santosh > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >
On 09/24/2015 09:05 AM, Murali Karicheri wrote: > On 09/23/2015 02:19 PM, santosh shilimkar wrote: >> Nishant, >> >> On 9/22/2015 9:08 AM, Nishanth Menon wrote: >>> Keystone2 devices are used on more platforms than just Texas >>> Instruments reference evaluation platforms called EVMs. Providing a >>> generic compatible "ti,keystone" is not sufficient to differentiate >>> various SoC definitions possible on various platforms. So, provide >>> compatible matches for each SoC family by itself. >>> >>> This allows SoC specific logic to be run time handled based on >>> of_machine_is_compatible("ti,k2hk") or as needed for the dependent >>> processor instead of needing to use board dependent compatibles that >>> are needed now. >>> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> --- >> You need to expand that 'not sufficient' for me. Unless there is >> genuine case to support this, I would want to avoid this churn. >> > > I agree. If there are run time check required in code to treat the > variants of Keystone SoC differently, then this change is needed. At > this time, SoC DTS captures these differences. IMHO, If a future > Keystone SoC support required SoC specific compatibility string to > customize SoC specific initialization code, then it can be introduced at > that time. I am not sure why this is introduced with out an example usage. a) See https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/usage-model.txt#n116 -> all we have today is board, soc generic family. The generic compatible flag of ti,keystone to mark that this is a keystone2 device does not identify that what kind of soc it is. which is what this series introduces. b) there is no realistic way for user space applications such as a test framework or other SoC based applications in a generic distro to detect the right SoC this platform is running on - imagine writing SoC specific code that depends on board compatible flags - there is never an end to that story as the number of boards expand. c) Ideally we will try never to introduce code that will have to depend on SoC compatible flag based match in kernel - but that is not a reason not to follow guidelines provided by devicetree documentation to provide SoC specific compatible flags.
Nishanth, On 09/24/2015 10:20 AM, Nishanth Menon wrote: > On 09/24/2015 09:05 AM, Murali Karicheri wrote: >> On 09/23/2015 02:19 PM, santosh shilimkar wrote: >>> Nishant, >>> >>> On 9/22/2015 9:08 AM, Nishanth Menon wrote: >>>> Keystone2 devices are used on more platforms than just Texas >>>> Instruments reference evaluation platforms called EVMs. Providing a >>>> generic compatible "ti,keystone" is not sufficient to differentiate >>>> various SoC definitions possible on various platforms. So, provide >>>> compatible matches for each SoC family by itself. >>>> >>>> This allows SoC specific logic to be run time handled based on >>>> of_machine_is_compatible("ti,k2hk") or as needed for the dependent >>>> processor instead of needing to use board dependent compatibles that >>>> are needed now. >>>> >>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>> --- >>> You need to expand that 'not sufficient' for me. Unless there is >>> genuine case to support this, I would want to avoid this churn. >>> >> >> I agree. If there are run time check required in code to treat the >> variants of Keystone SoC differently, then this change is needed. At >> this time, SoC DTS captures these differences. IMHO, If a future >> Keystone SoC support required SoC specific compatibility string to >> customize SoC specific initialization code, then it can be introduced at >> that time. I am not sure why this is introduced with out an example usage. > > a) See > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/usage-model.txt#n116 > -> all we have today is board, soc generic family. The generic > compatible flag of ti,keystone to mark that this is a keystone2 device > does not identify that what kind of soc it is. which is what this > series introduces. > > b) there is no realistic way for user space applications such as a > test framework or other SoC based applications in a generic distro to > detect the right SoC this platform is running on - imagine writing SoC > specific code that depends on board compatible flags - there is never > an end to that story as the number of boards expand. > > c) Ideally we will try never to introduce code that will have to > depend on SoC compatible flag based match in kernel - but that is not > a reason not to follow guidelines provided by devicetree documentation > to provide SoC specific compatible flags. > Thanks for pointing out the link which provided me some thing to cross check my argument. The relevant description from the above quoted below for discussion. ===================== Cut from the above =============================== The 'compatible' property contains a sorted list of strings starting with the exact name of the machine, followed by an optional list of boards it is compatible with sorted from most compatible to least. For example, the root compatible properties for the TI BeagleBoard and its successor, the BeagleBoard xM board might look like, respectively: compatible = "ti,omap3-beagleboard", "ti,omap3450", "ti,omap3"; compatible = "ti,omap3-beagleboard-xm", "ti,omap3450", "ti,omap3"; Where "ti,omap3-beagleboard-xm" specifies the exact model, it also claims that it compatible with the OMAP 3450 SoC, and the omap3 family of SoCs in general. You'll notice that the list is sorted from most specific (exact board) to least specific (SoC family). ======================================================================= So in keystone case, we have "ti,k2hk-evm","ti,keystone"; IMO, this is consistent with the above description as ti,k2hk-evm describes the board exactly and ti,keystone is the optional list which is a generic SoC name. =========================== Cut from the above========================== The reasoning behind this scheme is the observation that in the majority of cases, a single machine_desc can support a large number of boards if they all use the same SoC, or same family of SoCs. However, invariably there will be some exceptions where a specific board will require special setup code that is not useful in the generic case. Special cases could be handled by explicitly checking for the troublesome board(s) in generic setup code, but doing so very quickly becomes ugly and/or unmaintainable if it is more than just a couple of cases. Instead, the compatible list allows a generic machine_desc to provide support for a wide common set of boards by specifying "less compatible" values in the dt_compat list. In the example above, generic board support can claim compatibility with "ti,omap3" or "ti,omap3450". If a bug was discovered on the original beagleboard that required special workaround code during early boot, then a new machine_desc could be added which implements the workarounds and only matches on "ti,omap3-beagleboard". ======================================================================= This is how interpret the above. ti,omap3 is the family of omap3 devices similar to keystone. ti,omap3450 is required if there is an exceptional treatment required for ti,omap3450. In keystone case so far there is no case of exceptional treatment required in the code for a specific SoC. So a generic name, ti,keystone is used. When exceptional treatment is needed in the future, for example k2hk Soc, we should introduce SoC specific string in the following order. "ti,k2hk-evm", "ti,k2hk", "ti,keystone" So unless there is an exception, there is no need for a SoC specific string in the compatibility string list. So this can be added later if there is need for exceptional treatment. Did I get it wrong?
On 09/24/2015 10:54 AM, Murali Karicheri wrote: [...] > ti,omap3 is the family of omap3 devices similar to keystone. ti,omap3450 > is required if there is an exceptional treatment required for ti,omap3450. > > In keystone case so far there is no case of exceptional treatment > required in the code for a specific SoC. So a generic name, ti,keystone > is used. When exceptional treatment is needed in the future, for example > k2hk Soc, we should introduce SoC specific string in the following order. Did you do a grep on the code to see? $ git grep ti,omap3 arch/arm/mach-omap2/ arch/arm/mach-omap2/board-generic.c: "ti,omap3430", arch/arm/mach-omap2/board-generic.c: "ti,omap3", arch/arm/mach-omap2/board-generic.c: "ti,omap36xx", arch/arm/mach-omap2/board-generic.c: "ti,omap3-beagle", This is the same as keystone's device support. even though only 36xx was needed, we introduced other SoC specific compatibility match. > "ti,k2hk-evm", "ti,k2hk", "ti,keystone" > > So unless there is an exception, there is no need for a SoC specific > string in the compatibility string list. So this can be added later if > there is need for exceptional treatment. Did I get it wrong? > I see both your views seem to be "if we dont need a compatible" dont add it. My view was based on "be accurate in the hardware description" OK - i will probably agree on the topic. But, how about userspace needing to know which SoC they are on, without needing to depend on board->soc mapping? How do we help resolve that?
On 9/25/2015 7:50 AM, Nishanth Menon wrote: > On 09/24/2015 10:54 AM, Murali Karicheri wrote: > [...] >> ti,omap3 is the family of omap3 devices similar to keystone. ti,omap3450 >> is required if there is an exceptional treatment required for ti,omap3450. >> >> In keystone case so far there is no case of exceptional treatment >> required in the code for a specific SoC. So a generic name, ti,keystone >> is used. When exceptional treatment is needed in the future, for example >> k2hk Soc, we should introduce SoC specific string in the following order. > > Did you do a grep on the code to see? > $ git grep ti,omap3 arch/arm/mach-omap2/ > arch/arm/mach-omap2/board-generic.c: "ti,omap3430", > arch/arm/mach-omap2/board-generic.c: "ti,omap3", > arch/arm/mach-omap2/board-generic.c: "ti,omap36xx", > arch/arm/mach-omap2/board-generic.c: "ti,omap3-beagle", > > This is the same as keystone's device support. even though only 36xx was > needed, we introduced other SoC specific compatibility match. > >> "ti,k2hk-evm", "ti,k2hk", "ti,keystone" >> >> So unless there is an exception, there is no need for a SoC specific >> string in the compatibility string list. So this can be added later if >> there is need for exceptional treatment. Did I get it wrong? >> > > I see both your views seem to be "if we dont need a compatible" dont add > it. My view was based on "be accurate in the hardware description" > > OK - i will probably agree on the topic. But, how about userspace > needing to know which SoC they are on, without needing to depend on > board->soc mapping? How do we help resolve that? > Why the user space should care about exact SOC ?
On 09/25/2015 10:18 AM, santosh shilimkar wrote: > On 9/25/2015 7:50 AM, Nishanth Menon wrote: [...] >> But, how about userspace >> needing to know which SoC they are on, without needing to depend on >> board->soc mapping? How do we help resolve that? >> > Why the user space should care about exact SOC ? examples vary - trivial one is: debug tools like omapconf[1] or testing tools like opentest[2] need some standard way to ensure Linux kernel is functional - trusting the least set of parameters is usually what we would prefer. while building a generic distro such as debian or yocto, one prefers NOT to need to do a package build per SoC/perboard - that never scales. instead, you'd like the same application run on different systems dynamically. [1] https://github.com/omapconf/omapconf [2] http://arago-project.org/wiki/index.php/Opentest
9/25/2015 9:01 AM, Nishanth Menon wrote: > On 09/25/2015 10:18 AM, santosh shilimkar wrote: >> On 9/25/2015 7:50 AM, Nishanth Menon wrote: > [...] >>> But, how about userspace >>> needing to know which SoC they are on, without needing to depend on >>> board->soc mapping? How do we help resolve that? >>> >> Why the user space should care about exact SOC ? > > examples vary - trivial one is: debug tools like omapconf[1] or testing > tools like opentest[2] need some standard way to ensure Linux kernel is > functional - trusting the least set of parameters is usually what we > would prefer. while building a generic distro such as debian or yocto, > one prefers NOT to need to do a package build per SoC/perboard - that > never scales. instead, you'd like the same application run on different > systems dynamically. > I guessed omapconf example is coming though Keystone has no such tool yet ;-). Opentest shouldn't need that info either. I do agree that having a soc along with board is useful in longer run to accommodation more boards and variants. And only on that merit, I am willing to take these patches. Please refresh the series commit messages based on the discussion so far and repost. Will pick it up then. Regards, Santosh
On 09/25/2015 11:15 AM, santosh shilimkar wrote: > 9/25/2015 9:01 AM, Nishanth Menon wrote: >> On 09/25/2015 10:18 AM, santosh shilimkar wrote: >>> On 9/25/2015 7:50 AM, Nishanth Menon wrote: >> [...] >>>> But, how about userspace >>>> needing to know which SoC they are on, without needing to depend on >>>> board->soc mapping? How do we help resolve that? >>>> >>> Why the user space should care about exact SOC ? >> >> examples vary - trivial one is: debug tools like omapconf[1] or testing >> tools like opentest[2] need some standard way to ensure Linux kernel is >> functional - trusting the least set of parameters is usually what we >> would prefer. while building a generic distro such as debian or yocto, >> one prefers NOT to need to do a package build per SoC/perboard - that >> never scales. instead, you'd like the same application run on different >> systems dynamically. >> > I guessed omapconf example is coming though Keystone has no such tool :) That is true - as of now. maynot be the case for future. > yet ;-). Opentest shouldn't need that info either. we did debate this on opentest, but we could not implement anything since we did not have a consistent solution yet. > I do agree that having a soc along with board is useful in > longer run to accommodation more boards and variants. > And only on that merit, I am willing to take these patches. > > Please refresh the series commit messages based on the > discussion so far and repost. Will pick it up then. > Thanks. I will do so (probably early next week)..
On 09/25/2015 12:01 PM, Nishanth Menon wrote: > On 09/25/2015 10:18 AM, santosh shilimkar wrote: >> On 9/25/2015 7:50 AM, Nishanth Menon wrote: > [...] >>> But, how about userspace >>> needing to know which SoC they are on, without needing to depend on >>> board->soc mapping? How do we help resolve that? Believe it or not, user space tools that are custom to a specific SoC would require such knowledge. So I agree on this front that a SoC specific compatibility is cool to have. I think that should have been clear in the commit description. My Acked-By: Murali Karicheri <m-karicheri2@ti.com> >>> >> Why the user space should care about exact SOC ? > > examples vary - trivial one is: debug tools like omapconf[1] or testing > tools like opentest[2] need some standard way to ensure Linux kernel is > functional - trusting the least set of parameters is usually what we > would prefer. while building a generic distro such as debian or yocto, > one prefers NOT to need to do a package build per SoC/perboard - that > never scales. instead, you'd like the same application run on different > systems dynamically. > > > [1] https://github.com/omapconf/omapconf > [2] http://arago-project.org/wiki/index.php/Opentest >
Nishant, On 9/25/2015 10:38 AM, Nishanth Menon wrote: > On 09/25/2015 11:15 AM, santosh shilimkar wrote: >> 9/25/2015 9:01 AM, Nishanth Menon wrote: [..] >> Please refresh the series commit messages based on the >> discussion so far and repost. Will pick it up then. >> > Thanks. I will do so (probably early next week).. > Just want to check if you have series ready. I was planning to make queue ready over the weekend for upcoming merge window. I can hold that for another week though so let me know. Regards, Santosh
On 10/02/2015 11:09 AM, santosh shilimkar wrote: > Nishant, > > On 9/25/2015 10:38 AM, Nishanth Menon wrote: >> On 09/25/2015 11:15 AM, santosh shilimkar wrote: >>> 9/25/2015 9:01 AM, Nishanth Menon wrote: > > [..] > >>> Please refresh the series commit messages based on the >>> discussion so far and repost. Will pick it up then. >>> >> Thanks. I will do so (probably early next week).. >> > Just want to check if you have series ready. I was planning > to make queue ready over the weekend for upcoming merge > window. I can hold that for another week though so let > me know. Apologies on the delay. Update series was send: https://patchwork.kernel.org/patch/7322821/ https://patchwork.kernel.org/patch/7322831/ https://patchwork.kernel.org/patch/7322841/
On 10/3/15 4:44 PM, Nishanth Menon wrote: > On 10/02/2015 11:09 AM, santosh shilimkar wrote: >> Nishant, >> >> On 9/25/2015 10:38 AM, Nishanth Menon wrote: >>> On 09/25/2015 11:15 AM, santosh shilimkar wrote: >>>> 9/25/2015 9:01 AM, Nishanth Menon wrote: >> >> [..] >> >>>> Please refresh the series commit messages based on the >>>> discussion so far and repost. Will pick it up then. >>>> >>> Thanks. I will do so (probably early next week).. >>> >> Just want to check if you have series ready. I was planning >> to make queue ready over the weekend for upcoming merge >> window. I can hold that for another week though so let >> me know. > > Apologies on the delay. Update series was send: > https://patchwork.kernel.org/patch/7322821/ > https://patchwork.kernel.org/patch/7322831/ > https://patchwork.kernel.org/patch/7322841/ > No worries. Should show up in linux-next soon. I will let it sit there along with other patches for few more days and then send it up. Regards, Santosh
diff --git a/Documentation/devicetree/bindings/arm/keystone/keystone.txt b/Documentation/devicetree/bindings/arm/keystone/keystone.txt index 59d7a46f85eb..800d2d02e27b 100644 --- a/Documentation/devicetree/bindings/arm/keystone/keystone.txt +++ b/Documentation/devicetree/bindings/arm/keystone/keystone.txt @@ -9,12 +9,26 @@ Required properties: the form "ti,keystone-*". Generic devices like gic, arch_timers, ns16550 type UART should use the specified compatible for those devices. +SoC families: + +- Keystone 2 generic SoC: + compatible = "ti,keystone" + +SoCs: + +- Keystone 2 Hawking/Kepler + compatible = ti,k2hk", "ti,keystone" +- Keystone 2 Lamarr + compatible = ti,k2l", "ti,keystone" +- Keystone 2 Edison + compatible = ti,k2e", "ti,keystone" + Boards: - Keystone 2 Hawking/Kepler EVM - compatible = "ti,k2hk-evm","ti,keystone" + compatible = "ti,k2hk-evm", "ti,k2hk", "ti,keystone" - Keystone 2 Lamarr EVM - compatible = "ti,k2l-evm","ti,keystone" + compatible = "ti,k2l-evm", "ti, k2l", "ti,keystone" - Keystone 2 Edison EVM - compatible = "ti,k2e-evm","ti,keystone" + compatible = "ti,k2e-evm", "ti,k2e", "ti,keystone"
Keystone2 devices are used on more platforms than just Texas Instruments reference evaluation platforms called EVMs. Providing a generic compatible "ti,keystone" is not sufficient to differentiate various SoC definitions possible on various platforms. So, provide compatible matches for each SoC family by itself. This allows SoC specific logic to be run time handled based on of_machine_is_compatible("ti,k2hk") or as needed for the dependent processor instead of needing to use board dependent compatibles that are needed now. Signed-off-by: Nishanth Menon <nm@ti.com> --- .../devicetree/bindings/arm/keystone/keystone.txt | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)