Message ID | 1492689397-4704-5-git-send-email-clg@kaod.org |
---|---|
State | Rejected |
Delegated to: | Cyrille Pitchen |
Headers | show |
On 04/20/2017 01:56 PM, Cédric Le Goater wrote: > When reading flash contents, try to use the "command mode" if the AHB > window configured for the flash module is big enough. Else, just fall > back to the "user mode" to perform the read. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Changes since initial version : > > - rebased on current patchset which removed DMA support > > drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c > index 60c020482946..43015aec4557 100644 > --- a/drivers/mtd/spi-nor/aspeed-smc.c > +++ b/drivers/mtd/spi-nor/aspeed-smc.c > @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, > > aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); > aspeed_smc_stop_user(nor); > + return 0; > +} > + > +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, > + u_char *read_buf) > +{ > + struct aspeed_smc_chip *chip = nor->priv; > + > + /* > + * The AHB window configured for the chip is too small for the > + * read offset. Use the "User mode" of the controller to > + * perform the read. > + */ > + if (from >= chip->ahb_window_size) { > + aspeed_smc_read_user(nor, from, len, read_buf); > + goto out; What about turning this into dumb if () {} else {} and dropping the goto ? > + } > + > + /* > + * Use the "Command mode" to do a direct read from the AHB > + * window configured for the chip. This should be the default. > + */ > + memcpy_fromio(read_buf, chip->ahb_base + from, len); > + > +out: > return len; > } > > @@ -817,7 +842,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, > nor->dev = dev; > nor->priv = chip; > spi_nor_set_flash_node(nor, child); > - nor->read = aspeed_smc_read_user; > + nor->read = aspeed_smc_read; > nor->write = aspeed_smc_write_user; > nor->read_reg = aspeed_smc_read_reg; > nor->write_reg = aspeed_smc_write_reg; >
Am 20.04.2017 um 15:31 schrieb Marek Vasut: > On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >> When reading flash contents, try to use the "command mode" if the AHB >> window configured for the flash module is big enough. Else, just fall >> back to the "user mode" to perform the read. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Changes since initial version : >> >> - rebased on current patchset which removed DMA support >> >> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >> index 60c020482946..43015aec4557 100644 >> --- a/drivers/mtd/spi-nor/aspeed-smc.c >> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >> >> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >> aspeed_smc_stop_user(nor); >> + return 0; >> +} >> + >> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >> + u_char *read_buf) >> +{ >> + struct aspeed_smc_chip *chip = nor->priv; >> + >> + /* >> + * The AHB window configured for the chip is too small for the >> + * read offset. Use the "User mode" of the controller to >> + * perform the read. >> + */ >> + if (from >= chip->ahb_window_size) { >> + aspeed_smc_read_user(nor, from, len, read_buf); >> + goto out; > > What about turning this into dumb if () {} else {} and dropping the goto ? This is a matter of taste... Thanks, //richard
On 04/20/2017 03:31 PM, Marek Vasut wrote: > On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >> When reading flash contents, try to use the "command mode" if the AHB >> window configured for the flash module is big enough. Else, just fall >> back to the "user mode" to perform the read. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Changes since initial version : >> >> - rebased on current patchset which removed DMA support >> >> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >> index 60c020482946..43015aec4557 100644 >> --- a/drivers/mtd/spi-nor/aspeed-smc.c >> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >> >> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >> aspeed_smc_stop_user(nor); >> + return 0; >> +} >> + >> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >> + u_char *read_buf) >> +{ >> + struct aspeed_smc_chip *chip = nor->priv; >> + >> + /* >> + * The AHB window configured for the chip is too small for the >> + * read offset. Use the "User mode" of the controller to >> + * perform the read. >> + */ >> + if (from >= chip->ahb_window_size) { >> + aspeed_smc_read_user(nor, from, len, read_buf); >> + goto out; > > What about turning this into dumb if () {} else {} and dropping the goto ? yes, well, this is because I have other patches adding DMAs : https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 but as of today, the benefits in speed are to be proven and the current implementation does not support vmalloc'ed buffers. So I can drop the goto without too much regrets (if you insist :) Cheers, C. >> + } >> + >> + /* >> + * Use the "Command mode" to do a direct read from the AHB >> + * window configured for the chip. This should be the default. >> + */ >> + memcpy_fromio(read_buf, chip->ahb_base + from, len); >> + >> +out: >> return len; >> } >> >> @@ -817,7 +842,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, >> nor->dev = dev; >> nor->priv = chip; >> spi_nor_set_flash_node(nor, child); >> - nor->read = aspeed_smc_read_user; >> + nor->read = aspeed_smc_read; >> nor->write = aspeed_smc_write_user; >> nor->read_reg = aspeed_smc_read_reg; >> nor->write_reg = aspeed_smc_write_reg; >> > >
On 04/20/2017 03:53 PM, Cédric Le Goater wrote: > On 04/20/2017 03:31 PM, Marek Vasut wrote: >> On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >>> When reading flash contents, try to use the "command mode" if the AHB >>> window configured for the flash module is big enough. Else, just fall >>> back to the "user mode" to perform the read. >>> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> >>> Changes since initial version : >>> >>> - rebased on current patchset which removed DMA support >>> >>> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >>> 1 file changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>> index 60c020482946..43015aec4557 100644 >>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>> >>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>> aspeed_smc_stop_user(nor); >>> + return 0; >>> +} >>> + >>> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >>> + u_char *read_buf) >>> +{ >>> + struct aspeed_smc_chip *chip = nor->priv; >>> + >>> + /* >>> + * The AHB window configured for the chip is too small for the >>> + * read offset. Use the "User mode" of the controller to >>> + * perform the read. >>> + */ >>> + if (from >= chip->ahb_window_size) { >>> + aspeed_smc_read_user(nor, from, len, read_buf); >>> + goto out; >> >> What about turning this into dumb if () {} else {} and dropping the goto ? > > yes, well, this is because I have other patches adding DMAs : > > https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 > > but as of today, the benefits in speed are to be proven and the current > implementation does not support vmalloc'ed buffers. So I can drop the > goto without too much regrets (if you insist :) Nah, if you have further plans with this code, I don't mind either way.
On 04/20/2017 03:58 PM, Marek Vasut wrote: > On 04/20/2017 03:53 PM, Cédric Le Goater wrote: >> On 04/20/2017 03:31 PM, Marek Vasut wrote: >>> On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >>>> When reading flash contents, try to use the "command mode" if the AHB >>>> window configured for the flash module is big enough. Else, just fall >>>> back to the "user mode" to perform the read. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> >>>> Changes since initial version : >>>> >>>> - rebased on current patchset which removed DMA support >>>> >>>> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >>>> 1 file changed, 26 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>>> index 60c020482946..43015aec4557 100644 >>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>>> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>>> >>>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>>> aspeed_smc_stop_user(nor); >>>> + return 0; >>>> +} >>>> + >>>> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >>>> + u_char *read_buf) >>>> +{ >>>> + struct aspeed_smc_chip *chip = nor->priv; >>>> + >>>> + /* >>>> + * The AHB window configured for the chip is too small for the >>>> + * read offset. Use the "User mode" of the controller to >>>> + * perform the read. >>>> + */ >>>> + if (from >= chip->ahb_window_size) { >>>> + aspeed_smc_read_user(nor, from, len, read_buf); >>>> + goto out; >>> >>> What about turning this into dumb if () {} else {} and dropping the goto ? >> >> yes, well, this is because I have other patches adding DMAs : >> >> https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 >> >> but as of today, the benefits in speed are to be proven and the current >> implementation does not support vmalloc'ed buffers. So I can drop the >> goto without too much regrets (if you insist :) > > Nah, if you have further plans with this code, I don't mind either way. > OK let's keep that way. Do you think we can merge these four patches in the next window ? Thanks, C.
[ fixing Cyrille email ] On 04/20/2017 03:58 PM, Marek Vasut wrote: > On 04/20/2017 03:53 PM, Cédric Le Goater wrote: >> On 04/20/2017 03:31 PM, Marek Vasut wrote: >>> On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >>>> When reading flash contents, try to use the "command mode" if the AHB >>>> window configured for the flash module is big enough. Else, just fall >>>> back to the "user mode" to perform the read. >>>> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> >>>> Changes since initial version : >>>> >>>> - rebased on current patchset which removed DMA support >>>> >>>> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >>>> 1 file changed, 26 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>>> index 60c020482946..43015aec4557 100644 >>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>>> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>>> >>>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>>> aspeed_smc_stop_user(nor); >>>> + return 0; >>>> +} >>>> + >>>> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >>>> + u_char *read_buf) >>>> +{ >>>> + struct aspeed_smc_chip *chip = nor->priv; >>>> + >>>> + /* >>>> + * The AHB window configured for the chip is too small for the >>>> + * read offset. Use the "User mode" of the controller to >>>> + * perform the read. >>>> + */ >>>> + if (from >= chip->ahb_window_size) { >>>> + aspeed_smc_read_user(nor, from, len, read_buf); >>>> + goto out; >>> >>> What about turning this into dumb if () {} else {} and dropping the goto ? >> >> yes, well, this is because I have other patches adding DMAs : >> >> https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 >> >> but as of today, the benefits in speed are to be proven and the current >> implementation does not support vmalloc'ed buffers. So I can drop the >> goto without too much regrets (if you insist :) > > Nah, if you have further plans with this code, I don't mind either way. > OK let's keep that way. Do you think we can merge these four patches in the next window ? Thanks, C.
On 06/20/2017 03:42 PM, Cyrille Pitchen wrote: > Hi Cédric, > > Le 20/06/2017 à 11:07, Cédric Le Goater a écrit : >> [ fixing Cyrille email ] >> >> On 04/20/2017 03:58 PM, Marek Vasut wrote: >>> On 04/20/2017 03:53 PM, Cédric Le Goater wrote: >>>> On 04/20/2017 03:31 PM, Marek Vasut wrote: >>>>> On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >>>>>> When reading flash contents, try to use the "command mode" if the AHB >>>>>> window configured for the flash module is big enough. Else, just fall >>>>>> back to the "user mode" to perform the read. >>>>>> >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>>>> --- >>>>>> >>>>>> Changes since initial version : >>>>>> >>>>>> - rebased on current patchset which removed DMA support >>>>>> >>>>>> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >>>>>> 1 file changed, 26 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> index 60c020482946..43015aec4557 100644 >>>>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>>>>> >>>>>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>>>>> aspeed_smc_stop_user(nor); >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >>>>>> + u_char *read_buf) >>>>>> +{ >>>>>> + struct aspeed_smc_chip *chip = nor->priv; >>>>>> + >>>>>> + /* >>>>>> + * The AHB window configured for the chip is too small for the >>>>>> + * read offset. Use the "User mode" of the controller to >>>>>> + * perform the read. >>>>>> + */ >>>>>> + if (from >= chip->ahb_window_size) { >>>>>> + aspeed_smc_read_user(nor, from, len, read_buf); >>>>>> + goto out; >>>>> >>>>> What about turning this into dumb if () {} else {} and dropping the goto ? >>>> >>>> yes, well, this is because I have other patches adding DMAs : >>>> >>>> https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 >>>> >>>> but as of today, the benefits in speed are to be proven and the current >>>> implementation does not support vmalloc'ed buffers. So I can drop the >>>> goto without too much regrets (if you insist :) >>> >>> Nah, if you have further plans with this code, I don't mind either way. >>> >> >> OK let's keep that way. >> >> Do you think we can merge these four patches in the next window ? >> > > I've planned to merge them in the spi-nor/next branch of l2-mtd within > the next days if everything is OK. That's the git repo : git://github.com/spi-nor/linux.git ? > Last Friday I saw on patchwork that your patches were still waiting to > be merged. I've just forgotten them sorry and last week-end I was not > available to catch up. > > I add a quick overview of the series, everything looks good. patch 2 > won't apply as is since flash_read no longer exists in 'struct spi_nor' > so I will try to adapt your patch myself if I can. Ah. I work on mainline generally, so I missed it. > Otherwise I will ask you help to rebase it but I guess the modification > will be quite straight forward. Sure. Please don't hesitate to ping me. I will take a quick look anyway. Thanks, C.
[ fixing Cyrille email. your "From" is still @atmel.com. Am I doing something wrong ? ] On 06/20/2017 03:42 PM, Cyrille Pitchen wrote: > Hi Cédric, > > Le 20/06/2017 à 11:07, Cédric Le Goater a écrit : >> [ fixing Cyrille email ] >> >> On 04/20/2017 03:58 PM, Marek Vasut wrote: >>> On 04/20/2017 03:53 PM, Cédric Le Goater wrote: >>>> On 04/20/2017 03:31 PM, Marek Vasut wrote: >>>>> On 04/20/2017 01:56 PM, Cédric Le Goater wrote: >>>>>> When reading flash contents, try to use the "command mode" if the AHB >>>>>> window configured for the flash module is big enough. Else, just fall >>>>>> back to the "user mode" to perform the read. >>>>>> >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>>>> --- >>>>>> >>>>>> Changes since initial version : >>>>>> >>>>>> - rebased on current patchset which removed DMA support >>>>>> >>>>>> drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- >>>>>> 1 file changed, 26 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> index 60c020482946..43015aec4557 100644 >>>>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>>>>> @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>>>>> >>>>>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>>>>> aspeed_smc_stop_user(nor); >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, >>>>>> + u_char *read_buf) >>>>>> +{ >>>>>> + struct aspeed_smc_chip *chip = nor->priv; >>>>>> + >>>>>> + /* >>>>>> + * The AHB window configured for the chip is too small for the >>>>>> + * read offset. Use the "User mode" of the controller to >>>>>> + * perform the read. >>>>>> + */ >>>>>> + if (from >= chip->ahb_window_size) { >>>>>> + aspeed_smc_read_user(nor, from, len, read_buf); >>>>>> + goto out; >>>>> >>>>> What about turning this into dumb if () {} else {} and dropping the goto ? >>>> >>>> yes, well, this is because I have other patches adding DMAs : >>>> >>>> https://github.com/legoater/linux/commit/6880924dc2cf7a47c446a708df528b72d9bf9134#diff-e466368d609006970aab53a7b840221fR575 >>>> >>>> but as of today, the benefits in speed are to be proven and the current >>>> implementation does not support vmalloc'ed buffers. So I can drop the >>>> goto without too much regrets (if you insist :) >>> >>> Nah, if you have further plans with this code, I don't mind either way. >>> >> >> OK let's keep that way. >> >> Do you think we can merge these four patches in the next window ? >> > > I've planned to merge them in the spi-nor/next branch of l2-mtd within > the next days if everything is OK. That's the git repo : git://github.com/spi-nor/linux.git ? > Last Friday I saw on patchwork that your patches were still waiting to > be merged. I've just forgotten them sorry and last week-end I was not > available to catch up. > > I add a quick overview of the series, everything looks good. patch 2 > won't apply as is since flash_read no longer exists in 'struct spi_nor' > so I will try to adapt your patch myself if I can. Ah. I work on mainline generally, so I missed it. > Otherwise I will ask you help to rebase it but I guess the modification > will be quite straight forward. Sure. Please don't hesitate to ping me. I will take a quick look anyway. Thanks, C.
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c index 60c020482946..43015aec4557 100644 --- a/drivers/mtd/spi-nor/aspeed-smc.c +++ b/drivers/mtd/spi-nor/aspeed-smc.c @@ -394,6 +394,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); aspeed_smc_stop_user(nor); + return 0; +} + +static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len, + u_char *read_buf) +{ + struct aspeed_smc_chip *chip = nor->priv; + + /* + * The AHB window configured for the chip is too small for the + * read offset. Use the "User mode" of the controller to + * perform the read. + */ + if (from >= chip->ahb_window_size) { + aspeed_smc_read_user(nor, from, len, read_buf); + goto out; + } + + /* + * Use the "Command mode" to do a direct read from the AHB + * window configured for the chip. This should be the default. + */ + memcpy_fromio(read_buf, chip->ahb_base + from, len); + +out: return len; } @@ -817,7 +842,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, nor->dev = dev; nor->priv = chip; spi_nor_set_flash_node(nor, child); - nor->read = aspeed_smc_read_user; + nor->read = aspeed_smc_read; nor->write = aspeed_smc_write_user; nor->read_reg = aspeed_smc_read_reg; nor->write_reg = aspeed_smc_write_reg;
When reading flash contents, try to use the "command mode" if the AHB window configured for the flash module is big enough. Else, just fall back to the "user mode" to perform the read. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Changes since initial version : - rebased on current patchset which removed DMA support drivers/mtd/spi-nor/aspeed-smc.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)