linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* ASMedia PMP inquiry
@ 2024-03-18 11:32 Niklas Cassel
  0 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-03-18 11:32 UTC (permalink / raw)
  To: Szuying Chen
  Cc: dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, temnota.am, hdegoede, Ioannis Barkas

Hello ASMedia developers,


We are facing some issues with some of your HBAs, e.g. ASM1166 and ASM1064.

The first problem was that e.g. ASM1064 reports more ports than the physical
number of ports, see:
https://bugzilla.kernel.org/show_bug.cgi?id=211873
https://bugzilla.kernel.org/show_bug.cgi?id=218346

This was fixed in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/ata?id=0077a504e1a4468669fd2e011108db49133db56e
and
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/ata?id=9815e39617541ef52d0dfac4be274ad378c6dc09


For some PCIe cards with ASM1064, like the Delock 90073 16 port sata controller.
(It contains 4x ASM1064, each connected to one PCIe lane.)

So the port map went from:
0xffff0f	: 1111 1111 1111 1111 0000 1111
to
0xf		:                          1111

These virtual ports, which are not backed by a physical port,
causes the boot time to be significantly increased (by 3-4 minutes),
waiting for timeouts on these virtual ports.

From reading the ASM1064 datasheet, it only seems to support 4 ports,
so it seems like a AHCI spec violation to define more ports than what
is physically provided by the HBA silicon.

For other PCIe cards with ASM1064, e.g.
https://www.ebay.com/itm/204230341609

There are 4x JMicron JMB575 Port Multipliers (PMP) embedded on the PCIe
card itself (presumably one for each physical port on the ASM1064).
For this card, the virtual ports do not seem to cause an increased boot
time.

Both of these cards have the same PCIe device and vendor IDs (the ASMedia one),
so it is not trivial to quirk them based on that.


Looking at the boot logs for ASM1064 (the one with the JMB575 PMPs) on
kernel v6.8:
ahci 0000:04:00.0: AHCI 0001.0301 32 slots 24 ports 6 Gbps 0xf impl SATA mode
ahci 0000:04:00.0: flags: 64bit ncq sntf stag pm led only pio sxs deso sadm sds apst

We can see that the HBA does not advertise PMP support.
CAP.SPM (Supports Port Multiplier)) is not set.

According to the ASM1064 datasheet, the HBA is supposed to support PMP,
so this seems to be another instance where the AHCI specification
is not followed.

If we quirk the ASM1064, to set the CAP.SPM bit in CAP, we still
can not detect a PMP on port0-port3:

ahci 0000:04:00.0: ASM1064 has only four ports
ahci 0000:04:00.0: controller can do PMP, turning on CAP_PMP
ahci 0000:04:00.0: forcing port_map 0xffff0f -> 0xf
ahci 0000:04:00.0: AHCI 0001.0301 32 slots 24 ports 6 Gbps 0xf impl SATA mode
ahci 0000:04:00.0: flags: 64bit ncq sntf stag pm led only pmp pio sxs deso sadm sds apst

ata7: SATA max UDMA/133 abar m8192@0xfcd80000 port 0xfcd80100 irq 41 lpm-pol 0
ata8: SATA max UDMA/133 abar m8192@0xfcd80000 port 0xfcd80180 irq 41 lpm-pol 0
ata9: SATA max UDMA/133 abar m8192@0xfcd80000 port 0xfcd80200 irq 41 lpm-pol 0
ata10: SATA max UDMA/133 abar m8192@0xfcd80000 port 0xfcd80280 irq 41 lpm-pol 0

[    0.919020] ata7: SATA link down (SStatus 0 SControl 330)
[    2.787201] ata8: SATA link down (SStatus 0 SControl 330)
[    3.100522] ata9: SATA link down (SStatus 0 SControl 330)
[    3.413890] ata10: SATA link down (SStatus 0 SControl 330)

I was expecting link up and PMP class code detected on port0-port3.
On which ports are the PMPs connected then?
How are you supposed to talk to the PMP (i.e. send PMP read and PMP write
commands).

We need to be able to read out the PMP device and vendor ID of the PMP, and
to enumerate the devices behind the PMP according to AHCI and SATA-IO specs.


It appears that ASM1064 is violating these specifications, and instead being
able to enumerate the devices behind a PMP according to the specs, ASM1064
firmware tries to hide the fact that a PMP is connected to one of the ports.

A third example is here:
https://lore.kernel.org/linux-ide/CADUzMVaFcD26QiBK_eKCbtC5Ot-+hAruNbUx+2pQNTKtMhDGRA@mail.gmail.com/

A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
We need to be able to read the PMP device and vendor ID, in order to apply
the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
that is connected is bad, not only because it violates the specs, but also
because it stops us from providing the proper quirks for the connected PMP.

Could you please tell us how we can communicate with the PMPs directly?
(Regardless if they are external PMPs or PMPs embedded on the PCIe card.)


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
@ 2024-03-19  9:13 Szuying Chen
  2024-03-19 10:14 ` Andrey Jr. Melnikov
  2024-03-19 10:22 ` Niklas Cassel
  0 siblings, 2 replies; 12+ messages in thread
From: Szuying Chen @ 2024-03-19  9:13 UTC (permalink / raw)
  To: cassel
  Cc: dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, temnota.am, hdegoede, jnyb.de

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
---

On 3/18/24 19:32, Niklas Cassel wrote:
> A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> We need to be able to read the PMP device and vendor ID, in order to apply
> the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> that is connected is bad, not only because it violates the specs, but also
> because it stops us from providing the proper quirks for the connected PMP.
> 
> Could you please tell us how we can communicate with the PMPs directly?
> (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> 
Hello Niklas,

Unfortunately, our design does not provide interface to communicate with
the PMPs directly.
When ASM1064 plugging in an external PMP, we will hide the PMP and map to 
the virtual port to run.

Thanks.
Chloe

> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
@ 2024-03-19  9:28 Szuying Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Szuying Chen @ 2024-03-19  9:28 UTC (permalink / raw)
  To: cassel
  Cc: dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, temnota.am, hdegoede, jnyb.de

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
---

On 3/18/24 19:32, Niklas Cassel wrote:
> A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> We need to be able to read the PMP device and vendor ID, in order to apply
> the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> that is connected is bad, not only because it violates the specs, but also
> because it stops us from providing the proper quirks for the connected PMP.
> 
> Could you please tell us how we can communicate with the PMPs directly?
> (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> 
Hello Niklas,

Unfortunately, our design does not provide interface to communicate with
the PMPs directly.
When ASM1064 plugging in an external PMP, we will hide the PMP and map to 
the virtual port to run.

Thanks.
Chloe

> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
@ 2024-03-19  9:41 Szuying Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Szuying Chen @ 2024-03-19  9:41 UTC (permalink / raw)
  To: cassel, dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, temnota.am, hdegoede, jnyb.de

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
---

On 3/18/24 19:32, Niklas Cassel wrote:
> A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> We need to be able to read the PMP device and vendor ID, in order to apply
> the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> that is connected is bad, not only because it violates the specs, but also
> because it stops us from providing the proper quirks for the connected PMP.
> 
> Could you please tell us how we can communicate with the PMPs directly?
> (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> 
Hello Niklas,

Unfortunately, our design does not provide interface to communicate with
the PMPs directly.
When ASM1064 plugging in an external PMP, we will hide the PMP and map to 
the virtual port to run.

Thanks.
Chloe

> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-03-19  9:13 ASMedia PMP inquiry Szuying Chen
@ 2024-03-19 10:14 ` Andrey Jr. Melnikov
  2024-03-19 10:22 ` Niklas Cassel
  1 sibling, 0 replies; 12+ messages in thread
From: Andrey Jr. Melnikov @ 2024-03-19 10:14 UTC (permalink / raw)
  To: Szuying Chen
  Cc: cassel, dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, hdegoede, jnyb.de

On Tue, Mar 19, 2024 at 05:13:22PM +0800, Szuying Chen wrote:
> Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> ---
> 
> On 3/18/24 19:32, Niklas Cassel wrote:
> > A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> > We need to be able to read the PMP device and vendor ID, in order to apply
> > the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> > that is connected is bad, not only because it violates the specs, but also
> > because it stops us from providing the proper quirks for the connected PMP.
> > 
> > Could you please tell us how we can communicate with the PMPs directly?
> > (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> > 
> Hello Niklas,
> 
> Unfortunately, our design does not provide interface to communicate with
> the PMPs directly.
> When ASM1064 plugging in an external PMP, we will hide the PMP and map to 
> the virtual port to run.

How is possible detect such configuration?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-03-19  9:13 ASMedia PMP inquiry Szuying Chen
  2024-03-19 10:14 ` Andrey Jr. Melnikov
@ 2024-03-19 10:22 ` Niklas Cassel
  2024-03-20 19:35   ` Cryptearth
  2024-03-21 23:23   ` Re[2]: " Conrad Kostecki
  1 sibling, 2 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-03-19 10:22 UTC (permalink / raw)
  To: Szuying Chen
  Cc: dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	conikost, cryptearth, temnota.am, hdegoede, jnyb.de

Hello Chloe,

On Tue, Mar 19, 2024 at 05:13:22PM +0800, Szuying Chen wrote:
> Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> ---
> 
> On 3/18/24 19:32, Niklas Cassel wrote:
> > A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> > We need to be able to read the PMP device and vendor ID, in order to apply
> > the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> > that is connected is bad, not only because it violates the specs, but also
> > because it stops us from providing the proper quirks for the connected PMP.
> > 
> > Could you please tell us how we can communicate with the PMPs directly?
> > (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> > 
> Hello Niklas,
> 
> Unfortunately, our design does not provide interface to communicate with
> the PMPs directly.
> When ASM1064 plugging in an external PMP, we will hide the PMP and map to 
> the virtual port to run.

Thank you for your reply!

If you have any idea on how those users with a ASM1064 card that does not
have any PMPs on the PCIe cards, e.g. Delock 90073, can avoid the extra
2-3 minutes delay during boot, we are open to suggestions.

I guess you could send a new firmware to Delock that sets the PI register
(Ports Implemented) register to 0xf.

However, from what I've understood from how you have decided to implement
PMP support on your HBAs, I assume that setting the PI register to 0xf
would also stop Delock 90073 from working with an externally connected
port multiplier, so that it probably not a good approach after all.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-03-19 10:22 ` Niklas Cassel
@ 2024-03-20 19:35   ` Cryptearth
  2024-03-21 23:23   ` Re[2]: " Conrad Kostecki
  1 sibling, 0 replies; 12+ messages in thread
From: Cryptearth @ 2024-03-20 19:35 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Szuying Chen, dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu,
	Chloe_Chen, conikost, temnota.am, hdegoede, jnyb.de

>
> Hello Chloe,
>
> On Tue, Mar 19, 2024 at 05:13:22PM +0800, Szuying Chen wrote:
> > Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> > ---
> >
> > On 3/18/24 19:32, Niklas Cassel wrote:
> > > A user plugging in an external PMP (so not a PMP embedded on the PCIe card).
> > > We need to be able to read the PMP device and vendor ID, in order to apply
> > > the correct PMP quirks, see sata_pmp_quirks(). So trying to hide which PMP
> > > that is connected is bad, not only because it violates the specs, but also
> > > because it stops us from providing the proper quirks for the connected PMP.
> > >
> > > Could you please tell us how we can communicate with the PMPs directly?
> > > (Regardless if they are external PMPs or PMPs embedded on the PCIe card.)
> > >
> > Hello Niklas,
> >
> > Unfortunately, our design does not provide interface to communicate with
> > the PMPs directly.
> > When ASM1064 plugging in an external PMP, we will hide the PMP and map to
> > the virtual port to run.
>
> Thank you for your reply!
>
> If you have any idea on how those users with a ASM1064 card that does not
> have any PMPs on the PCIe cards, e.g. Delock 90073, can avoid the extra
> 2-3 minutes delay during boot, we are open to suggestions.
>
> I guess you could send a new firmware to Delock that sets the PI register
> (Ports Implemented) register to 0xf.
>
> However, from what I've understood from how you have decided to implement
> PMP support on your HBAs, I assume that setting the PI register to 0xf
> would also stop Delock 90073 from working with an externally connected
> port multiplier, so that it probably not a good approach after all.
>
>
> Kind regards,
> Niklas

So, I guess the current conclusion is that there's no optimal solution
due to hardware not following the standards?
I also thought about the option to use override parameters like
setting the port mask. But then I also thought about: How to handle a
system with different HBAs?
Like a system using one card with a pci-e multiplexer but also a card
like mine with port multipliers. The result would be either the
current behaviour - all ports working but long timeouts - or like with
the change - no timeouts but not all ports working.
An optimal solution would requite a per-HBA setting. To keep a
consistent order the address path could be used, like "the first HBA
is on pci-e_3, the second on pci-e_5". It would still be up to the
user to correctly identify the correct ports and set up a working
kernel command but at least it would be an option for mixed systems to
set parameters per controller instead of applying the same setting
global.

I searched for both host controllers as well as port multipliers.
Although most solutions end up with controllers from ASMedia, JMicron
or even SLI there seem also way more lesser known chips out there.
I also searched for more details around all that topic - and was able
to find older forum threads from 2008 to 2013. So this seem to already
going on for several years (hence I not really understood the initial
report about the long boot timeout as I thought "well, people deal
with it for over a decade now").

Aside from the revert is in my favour I also see that there're some
deeper issues that either have to be ignored potentially breaking
hardware, tried to worked around with (would a separate driver be
possible?) or to get in contact with major controller manufactures to
get them to come up with changes for newer chips/firmware - this way
at least new hardware can have a chance to work out of the box by
following the standards.

Quite a complicated topic.

Matt

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re[2]: ASMedia PMP inquiry
  2024-03-19 10:22 ` Niklas Cassel
  2024-03-20 19:35   ` Cryptearth
@ 2024-03-21 23:23   ` Conrad Kostecki
  2024-03-22  0:00     ` Damien Le Moal
  1 sibling, 1 reply; 12+ messages in thread
From: Conrad Kostecki @ 2024-03-21 23:23 UTC (permalink / raw)
  To: Niklas Cassel, Szuying Chen
  Cc: dlemoal, linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen,
	cryptearth, temnota.am, hdegoede, jnyb.de

Hi Niklas,

Am 19.03.2024 11:22:26, "Niklas Cassel" <cassel@kernel.org> schrieb:

>However, from what I've understood from how you have decided to implement
>PMP support on your HBAs, I assume that setting the PI register to 0xf
>would also stop Delock 90073 from working with an externally connected
>port multiplier, so that it probably not a good approach after all.

I can see, that you have now reverted the changes / merged my patch with 
additional description.
Do you see any possibility for overriding saved_port_map as kernel 
parameter would be possibile for asm devices somehow?
Of course only as opt-in.

Conrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-03-21 23:23   ` Re[2]: " Conrad Kostecki
@ 2024-03-22  0:00     ` Damien Le Moal
  2024-04-02 18:31       ` Re[2]: " Conrad Kostecki
  0 siblings, 1 reply; 12+ messages in thread
From: Damien Le Moal @ 2024-03-22  0:00 UTC (permalink / raw)
  To: Conrad Kostecki, Niklas Cassel, Szuying Chen
  Cc: linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen, cryptearth,
	temnota.am, hdegoede, jnyb.de

On 3/22/24 08:23, Conrad Kostecki wrote:
> Hi Niklas,
> 
> Am 19.03.2024 11:22:26, "Niklas Cassel" <cassel@kernel.org> schrieb:
> 
>> However, from what I've understood from how you have decided to implement
>> PMP support on your HBAs, I assume that setting the PI register to 0xf
>> would also stop Delock 90073 from working with an externally connected
>> port multiplier, so that it probably not a good approach after all.
> 
> I can see, that you have now reverted the changes / merged my patch with 
> additional description.
> Do you see any possibility for overriding saved_port_map as kernel 
> parameter would be possibile for asm devices somehow?
> Of course only as opt-in.

Yes. I will add a libata.force parameter to do that so that setups that do not
use PMPs can avoid long boot times due to probing the broken "virtual ports".
Will post a patch next week for that.

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re[2]: ASMedia PMP inquiry
  2024-03-22  0:00     ` Damien Le Moal
@ 2024-04-02 18:31       ` Conrad Kostecki
  2024-04-02 23:21         ` Damien Le Moal
  2024-04-04  9:51         ` Damien Le Moal
  0 siblings, 2 replies; 12+ messages in thread
From: Conrad Kostecki @ 2024-04-02 18:31 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel, Szuying Chen
  Cc: linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen, cryptearth,
	temnota.am, hdegoede, jnyb.de

Hello Damien,

Am 22.03.2024 01:00:02, "Damien Le Moal" <dlemoal@kernel.org> schrieb:

>Yes. I will add a libata.force parameter to do that so that setups that do not
>use PMPs can avoid long boot times due to probing the broken "virtual ports".
>Will post a patch next week for that.
any news for the patch?

Cheers
Conrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-04-02 18:31       ` Re[2]: " Conrad Kostecki
@ 2024-04-02 23:21         ` Damien Le Moal
  2024-04-04  9:51         ` Damien Le Moal
  1 sibling, 0 replies; 12+ messages in thread
From: Damien Le Moal @ 2024-04-02 23:21 UTC (permalink / raw)
  To: Conrad Kostecki, Niklas Cassel, Szuying Chen
  Cc: linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen, cryptearth,
	temnota.am, hdegoede, jnyb.de

On 4/3/24 03:31, Conrad Kostecki wrote:
> Hello Damien,
> 
> Am 22.03.2024 01:00:02, "Damien Le Moal" <dlemoal@kernel.org> schrieb:
> 
>> Yes. I will add a libata.force parameter to do that so that setups that do not
>> use PMPs can avoid long boot times due to probing the broken "virtual ports".
>> Will post a patch next week for that.
> any news for the patch?

Been busy. I will get this started as soon as I can.

> 
> Cheers
> Conrad
> 

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ASMedia PMP inquiry
  2024-04-02 18:31       ` Re[2]: " Conrad Kostecki
  2024-04-02 23:21         ` Damien Le Moal
@ 2024-04-04  9:51         ` Damien Le Moal
  1 sibling, 0 replies; 12+ messages in thread
From: Damien Le Moal @ 2024-04-04  9:51 UTC (permalink / raw)
  To: Conrad Kostecki, Niklas Cassel, Szuying Chen
  Cc: linux-ide, Jesse1_Chang, Richard_Hsu, Chloe_Chen, cryptearth,
	temnota.am, hdegoede, jnyb.de

On 4/3/24 03:31, Conrad Kostecki wrote:
> Hello Damien,
> 
> Am 22.03.2024 01:00:02, "Damien Le Moal" <dlemoal@kernel.org> schrieb:
> 
>> Yes. I will add a libata.force parameter to do that so that setups that do not
>> use PMPs can avoid long boot times due to probing the broken "virtual ports".
>> Will post a patch next week for that.
> any news for the patch?

I just sent one, you are on cc.
If you could test, that would be great. Thanks.


-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-04-04  9:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-19  9:13 ASMedia PMP inquiry Szuying Chen
2024-03-19 10:14 ` Andrey Jr. Melnikov
2024-03-19 10:22 ` Niklas Cassel
2024-03-20 19:35   ` Cryptearth
2024-03-21 23:23   ` Re[2]: " Conrad Kostecki
2024-03-22  0:00     ` Damien Le Moal
2024-04-02 18:31       ` Re[2]: " Conrad Kostecki
2024-04-02 23:21         ` Damien Le Moal
2024-04-04  9:51         ` Damien Le Moal
  -- strict thread matches above, loose matches on Subject: below --
2024-03-19  9:41 Szuying Chen
2024-03-19  9:28 Szuying Chen
2024-03-18 11:32 Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).