> -----Original Message----- > From: Pavel Machek <pavel@xxxxxx> > Sent: Wednesday, October 21, 2020 11:34 AM > To: Vadim Pasternak <vadimp@xxxxxxxxxx> > Cc: Marek Behun <marek.behun@xxxxxx>; jacek.anaszewski@xxxxxxxxx; linux- > leds@xxxxxxxxxxxxxxx > Subject: Re: [PATCH led-next 1/1] leds: mlxreg: Allow multi-instantiation of > same name LED for modular systems > > Hi! > > > > > But why you consider it as function enumerator? > > > > For example card48, card56 are two different devices of same type. > > > > Both have 'status' LED. > > > > > > OK this is a fair point. > > > > > > I was thinking such because in my mind I had this idea that for an > > > ethernet switch with interfaces lan0 - lan4 it would make sense to > > > use the LED_FUNCTION_LAN function with function enumerator. But > > > thinking about this now again makes me wonder if instead the lan0 - > > > lan4 should be devicenames instead, since normally they are network > interface names. > > > > > > Vadim, the reason why Pavel and I think that mlxreg (or mlxregN) is > > > not valid devicename part (although mlxreg has to stay since many > > > users already depend on it, as you say), is that the mlxreg name is > > > not exposed anywhere else in Linux from userspace point of view. > > > > > > Devicename eth0 is okay, because it is network interface name. > > > Devicename sda would be okay, because everyone knows it is a block > > > device and you can access it via /dev/sda. > > > Devicename hci0 would be okay because it is bluetooth interface > > > accessible via hcitool. > > > Devicenames mtd0, kbd0, mouse0 would be okay, I think. > > > > > > But mlxreg is not accessible via anything else in the system. Unless > > > your systems also have something like /dev/mlxreg, that is. > > > > > > Do the LEDs on these cards only indicate status of the cards > > > themselves as a whole? Or are there LEDs on these cards dedicated to > > > their peripherals? For example if there is an ethernet port with > > > LEDs on one of these cards, the devicename part for these LEDs > > > should be of the device of that ethernet port, not mlxreg... > > > > Hi Marek, > > > > Each line card must have 'status' LED, indicating status of line card itself. > > User can set non-green in case some there are some alarms on different > > devices, equipped on this line card. It can be set blink during line card > initialization. > > > > Line card could be equipped with UID LED. User can set this LED in > > order to find physical location of line card. Sometimes it's hard to > > see the sticker on chassis. > > > > Line card also equipped with per port LED, but those LEDs are handled by > FW. > > > > So, the device in this case is 'line card'. > > > > In my previous reply I suggest name 'fru' stands for the filed replaceable unit. > > This is not something, that is exposed in '/dev', but it describes any > > replaceable unit within the system. > > So.. you'd use the LED to locate right PCI card, or the LED would indicate that > whole card is failing, etc...? > > Could we use pci00:1b.0 as the device name? (same as lspci). Probably replace > : with _... > Hi Pavel, Yes, STATU and UID LED indicates whole line card status/location. Some line cards could be connected through PCIe, but not all. >From chassis management perspective they are connected by I2C. And CPLD register map is accessed through I2C. Following your suggestion it could be i2c-{n} as device name (pdev->id is I2C bus): if (priv->pdev->id > 0) sprintf(led_data->led_cdev_name, "%s%d:%s", "i2c-", priv->pdev->id, data->label); else sprintf(led_data->led_cdev_name, "%s:%s", "mlxreg", data->label); Or bus name could be passed through the 'identity' filed: if (priv->pdev->id > 0) sprintf(led_data->led_cdev_name, "%s%d:%s", led_pdata->identity, priv->pdev->id, data->label); else sprintf(led_data->led_cdev_name, "%s:%s", "mlxreg", data->label); Thanks, Vadim. > Best regards, > Pavel > -- > http://www.livejournal.com/~pavelmachek