RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > > In various parts of the enumeration and routing algorithm, it would need > to lookup the tag to find the destid, if the destid is in the tag then > this lookup is not needed. Let's keep this discussion within limits of the current implementation. Existing method of forming CT may be replaced when that change is justified by use. Any method that may be implemented later is good if it ensures unique identification of RIO devices. At this moment, we just need to define CT format now to avoid future conflicts of enumeration methods. > Well, we both know "the current mainline implementation" is easily > improved to unique identification I proposed. Therefore this statement > is misleading. Your improvement will require assigning separate destID to any switch that has no endpoints attached. I prefer to keep changes to enumeration for later patches. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen wrote: I look at it this way: it prevents the need for another layer of indirection: translating component tag to a destid. The destid alone is not enough. You will need an entire rio_dev object for that device anyway. ?? I did not say a rio_dev object is not needed; on the contrary, I do have it. In various parts of the enumeration and routing algorithm, it would need to lookup the tag to find the destid, if the destid is in the tag then this lookup is not needed. I think we are mixing two things together here. I understand your idea but do not see how it prevents me from having one common set of access coordinates for RIO devices (the starting point of our discussion). I'm trying to argue we do not want redundant identification in the first place unless absolutely necessary. Regardless of an implementation, having a way that ensures unified identification of switches by all processor boards is better than the current mainline implementation. Well, we both know "the current mainline implementation" is easily improved to unique identification I proposed. Therefore this statement is misleading. Methods of forming a component tag may differ but still serve the same purpose. Personally I prefer to avoid any link of device identification to the destid because it may not be as intuitive as it seems for large systems with hot-plug. I will discuss this with some of RIO TWG guys to get their opinion on the best approach. Please do, please elaborate: "may not be as intuitive" ... to what? for implementation of ...? I will make a patch that defines fields of component tag, probably just one for now - identification field. This will ensure that any method used to assign component tag (id part of it) will be compatible with RIO spec part 8 error management. Again slightly misleading information. Any unique identification satisfies this requirement. To prove my point: are you going to recycle the component tags as well in case of hot-swaps, just like the destids? As for switch identification, at this moment I still prefer replacing rswitch->switchid with ID portion of the component tag because it is very simple and does not require changes to enumeration algorithm. Yes, I agree switchid = tag, that's what I use also. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > > I look at it this way: it prevents the need for another layer of > indirection: translating component tag to a destid. The destid alone is not enough. You will need an entire rio_dev object for that device anyway. > > Why no relation? My experience is that during debugging it's useful to > have the destid directly at hand, it's just very practical. (Otherwise > any drawing of a random network would need two "identification" numbers > per drawn node: the component tag (true identification), and destid > since that's what everyone uses to identify a device, what needs to > programmed into the LUTs of a switch, identification in sysfs, etc.). I think we are mixing two things together here. I understand your idea but do not see how it prevents me from having one common set of access coordinates for RIO devices (the starting point of our discussion). Regardless of an implementation, having a way that ensures unified identification of switches by all processor boards is better than the current mainline implementation. Methods of forming a component tag may differ but still serve the same purpose. Personally I prefer to avoid any link of device identification to the destid because it may not be as intuitive as it seems for large systems with hot-plug. I will discuss this with some of RIO TWG guys to get their opinion on the best approach. I will make a patch that defines fields of component tag, probably just one for now - identification field. This will ensure that any method used to assign component tag (id part of it) will be compatible with RIO spec part 8 error management. As for switch identification, at this moment I still prefer replacing rswitch->switchid with ID portion of the component tag because it is very simple and does not require changes to enumeration algorithm. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen wrote: that switch. The tag uses one extra bit to identify the device as a switch instead of an endpoint. This provides the information to unambiguously identify a switch from an endpoint. OK taking away #2. But do not see how it justifies storing two values of destid. I look at it this way: it prevents the need for another layer of indirection: translating component tag to a destid. And you have just confirmed using CT for unique identification. That's correct, but I never said (intended to say) I didn't. We simply have differences in interpretation of CT: you are using component tag to pass unique identification and I am using CT as a unique identification. I prefer not to assume any relationship between routing information and the component tag. Why no relation? My experience is that during debugging it's useful to have the destid directly at hand, it's just very practical. (Otherwise any drawing of a random network would need two "identification" numbers per drawn node: the component tag (true identification), and destid since that's what everyone uses to identify a device, what needs to programmed into the LUTs of a switch, identification in sysfs, etc.). Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > > Bounine, Alexandre wrote: > > 1. The destid for the switch needs an additional mechanism to share it > > among processors in the RIO network, > > ? See comment for 2) > > > 2. It takes ID value away from the pool of available IDs, what will > > It does not take an ID away, it shares it with a connected endpoint to > that switch. The tag uses one extra bit to identify the device as a > switch instead of an endpoint. This provides the information to > unambiguously identify a switch from an endpoint. OK taking away #2. But do not see how it justifies storing two values of destid. And you have just confirmed using CT for unique identification. We simply have differences in interpretation of CT: you are using component tag to pass unique identification and I am using CT as a unique identification. I prefer not to assume any relationship between routing information and the component tag. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen wrote: rid of rswitch->switchid and use component_tag directly for switches). I still prefer the destid as the single identification id. In your patch you allocate individual destid for switches. This method has two problems: 1. The destid for the switch needs an additional mechanism to share it among processors in the RIO network, ? See comment for 2) 2. It takes ID value away from the pool of available IDs, what will It does not take an ID away, it shares it with a connected endpoint to that switch. The tag uses one extra bit to identify the device as a switch instead of an endpoint. This provides the information to unambiguously identify a switch from an endpoint. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > Bounine, Alexandre wrote: > > If we will need to identify the same physical switch by different > > processors we may use the component tag which now is unique for every > > device. > > Yes, identification is the point. I think it might be confusing to have > a destid *and* a component tag id which are slightly different. The > destid is unambiguous (if you know whether the device is a switch or > endpoint) so I think it makes sense to use that if possible. The component tag is the way to identify a RIO device (switch or endpoint). 1. it is defined by RIO spec as a register existing in both types of devices (this provides a universal access to the identification information by any processor). 2. the Error Management specification already uses the CT as a device identifier. 3. the CT value is large enough to be unique for max number of endpoints in the large system and any reasonable number of switches. BTW, I am planning to provide a patch that defines CT fields to ensure future compatibility. The destid does not exists as a HW element of switches and therefore cannot be used as a universal identification token. > > This actually gives me another idea: instead of using global > > next_switchid counter make rswitch->switchid = component_tag and > > switches in sysfs will look identical for every processor (or just get > > rid of rswitch->switchid and use component_tag directly for switches). > > I still prefer the destid as the single identification id. As I answered above, destid cannot be used as a universal identification token - it is a routing element. The destID has register in endpoints only to provide a packet filtering. In your patch you allocate individual destid for switches. This method has two problems: 1. The destid for the switch needs an additional mechanism to share it among processors in the RIO network, 2. It takes ID value away from the pool of available IDs, what will reduce number of IDs that can be assigned to endpoints. (NOTE: I am actually working on destID assignment scheme that will recycle destID in case of hot-swap events, i.e. if device is extracted its destID will be returned to the pool of available IDs and may be reused later for device insertion). The only case when assigning individual destid to the switch is justified is an "empty" switch - one without any endpoints attached to it. But that destid should be assigned to an endpoint as soon as it is attached to that switch. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen wrote: rswitch->rdev->destid should be the id associated with a given switch, so that every (processor) device can agree what id some switch has. If we will need to identify the same physical switch by different processors we may use the component tag which now is unique for every device. Yes, identification is the point. I think it might be confusing to have a destid *and* a component tag id which are slightly different. The destid is unambiguous (if you know whether the device is a switch or endpoint) so I think it makes sense to use that if possible. This actually gives me another idea: instead of using global next_switchid counter make rswitch->switchid = component_tag and switches in sysfs will look identical for every processor (or just get rid of rswitch->switchid and use component_tag directly for switches). I still prefer the destid as the single identification id. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > > Bounine, Alexandre wrote: > > Looks like I formulated it bad - better would be: they have different > > interpretation by hardware but logically in RapidIO they have single > > role - destid/hopcount are a device coordinates in the RIO network used > > to access that device. > > They are logically different as well (for a non-host). > > rswitch->destid with hopcount is the way to reach that switch. > OK. This is moved to rdev->destid now to make access unified with endpoints. > rswitch->rdev->destid should be the id associated with a given switch, > so that every (processor) device can agree what id some switch has. For > a non-host, the path to reach a switch may use a different id than the > switch itself has; it's just the id by which it was discovered. > However, it's possible to fix that by fixing the id+hopcount once the > switch is found using the path with its own id: then you know the right > hopcount. I have got an impression that we are discussing slightly different implementations here. The suggested role of rswitch->rdev->destid is not clear to me. I agree that destid and hopcount for switch will be different for every processor. There is nothing wrong with it because a switch physically does not have its own ID. If we will need to identify the same physical switch by different processors we may use the component tag which now is unique for every device. This actually gives me another idea: instead of using global next_switchid counter make rswitch->switchid = component_tag and switches in sysfs will look identical for every processor (or just get rid of rswitch->switchid and use component_tag directly for switches). > >> can be defined to point to the switch that a given rio_dev is > > connected > >> to. This is useful for quick lookups. How else can to know to which > >> switch a given device is connected? > > > > rdev->rswitch is not a pointer to the entire switch device object - it > > is a pointer to the switch specific extension associated with given > > rio_dev (if applicable). There is no other role for rdev->rswitch. > > I know this, it doesn't answer my question. > > > Why would you keep a pointer to device data extension instead of the > > pointer to attached device object itself? > > There is no particular reason, but this is a useful way to define the > fields that are there. > > My point is, now that you remove the pointer field, that information (to > which switch is a particular device connected) cannot be stored in this > way, so do you have an alternative proposal for that? Maybe add a new field. > See my comment below ;). > > BTW, I have back and forward links added in previous patches and only > > one link that may be added later is a forward link from mport to the > > attached rio_dev (ptr to rio_switch will not work here because it can be > > switchless connection). But this reference has to be added into > > rio_mport. > > Possible, but I suggest to put it in the rio_net: fields rdev_host, and > rdev_self. You can see it in the patch I sent you. Yes, we may rework rio_net that way and use some good things from there. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen wrote: Alexandre Bounine wrote: How can you say this? The two variables have different meanings, this logically implies you can't merge them. So how do you say 'this does not prevent us from ...' without providing a reason? Looks like I formulated it bad - better would be: they have different interpretation by hardware but logically in RapidIO they have single role - destid/hopcount are a device coordinates in the RIO network used to access that device. They are logically different as well (for a non-host). rswitch->destid with hopcount is the way to reach that switch. rswitch->rdev->destid should be the id associated with a given switch, so that every (processor) device can agree what id some switch has. For a non-host, the path to reach a switch may use a different id than the switch itself has; it's just the id by which it was discovered. However, it's possible to fix that by fixing the id+hopcount once the switch is found using the path with its own id: then you know the right hopcount. can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? rdev->rswitch is not a pointer to the entire switch device object - it is a pointer to the switch specific extension associated with given rio_dev (if applicable). There is no other role for rdev->rswitch. I know this, it doesn't answer my question. Why would you keep a pointer to device data extension instead of the pointer to attached device object itself? There is no particular reason, but this is a useful way to define the fields that are there. My point is, now that you remove the pointer field, that information (to which switch is a particular device connected) cannot be stored in this way, so do you have an alternative proposal for that? Maybe add a new field. BTW, I have back and forward links added in previous patches and only one link that may be added later is a forward link from mport to the attached rio_dev (ptr to rio_switch will not work here because it can be switchless connection). But this reference has to be added into rio_mport. Possible, but I suggest to put it in the rio_net: fields rdev_host, and rdev_self. You can see it in the patch I sent you. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen wrote: > > Alexandre Bounine wrote: > > 1. Using one storage location common for switches and endpoints eliminates > > unnecessary device type checks during maintenance access operations. > > While destination IDs and hop counts have different meaning for endpoints and > > switches, this does not prevent us from storing them in the primary RIO device > > structure (rio_dev) for both types. > > How can you say this? The two variables have different meanings, this > logically implies you can't merge them. So how do you say 'this does not > prevent us from ...' without providing a reason? Looks like I formulated it bad - better would be: they have different interpretation by hardware but logically in RapidIO they have single role - destid/hopcount are a device coordinates in the RIO network used to access that device. > > 2. Convert RIO switch device structures (rio_dev + rio_switch) into single > > allocation unit. This change is based on the fact that RIO switches are using > > common RIO device objects anyway. Allocating RIO switch objects as RIO devices > > with added space for switch information simplifies handling of RIO switch device > > objects. > > I still don't think that's a good idea because the rdev->rswitch pointer > can be defined to point to the switch that a given rio_dev is connected > to. This is useful for quick lookups. How else can to know to which > switch a given device is connected? rdev->rswitch is not a pointer to the entire switch device object - it is a pointer to the switch specific extension associated with given rio_dev (if applicable). There is no other role for rdev->rswitch. Why would you keep a pointer to device data extension instead of the pointer to attached device object itself? BTW, I have back and forward links added in previous patches and only one link that may be added later is a forward link from mport to the attached rio_dev (ptr to rio_switch will not work here because it can be switchless connection). But this reference has to be added into rio_mport. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Alexandre Bounine wrote: 1. Using one storage location common for switches and endpoints eliminates unnecessary device type checks during maintenance access operations. While destination IDs and hop counts have different meaning for endpoints and switches, this does not prevent us from storing them in the primary RIO device structure (rio_dev) for both types. How can you say this? The two variables have different meanings, this logically implies you can't merge them. So how do you say 'this does not prevent us from ...' without providing a reason? 2. Convert RIO switch device structures (rio_dev + rio_switch) into single allocation unit. This change is based on the fact that RIO switches are using common RIO device objects anyway. Allocating RIO switch objects as RIO devices with added space for switch information simplifies handling of RIO switch device objects. I still don't think that's a good idea because the rdev->rswitch pointer can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
The following two patches are produced as result of the discussion referenced below: http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-September/085829.html http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-October/086226.html Switches in RapidIO subsystem are presented the same way as endpoints - by using rio_dev structure plus an additional switch-specific extension allocated separately. This separation between two objects describing a RIO switch device also is reflected in the way how RIO address is stored for endpoints and switches. Proposed patches are attempt to address issues brought by differences in endpoint and switch handling in RapidIO subsystem. 1. Using one storage location common for switches and endpoints eliminates unnecessary device type checks during maintenance access operations. While destination IDs and hop counts have different meaning for endpoints and switches, this does not prevent us from storing them in the primary RIO device structure (rio_dev) for both types. The logic that assigns destination IDs to RIO devices stays unchanged - as before, switches use an associated destination ID because they do not have their own physical ID. The hop_count is set to 0xff for endpoints and to the actual value for switches. 2. Convert RIO switch device structures (rio_dev + rio_switch) into single allocation unit. This change is based on the fact that RIO switches are using common RIO device objects anyway. Allocating RIO switch objects as RIO devices with added space for switch information simplifies handling of RIO switch device objects. Alexandre Bounine (2): RapidIO: Use common destid storage for endpoints and switches RapidIO: Integrate rio_switch into rio_dev drivers/rapidio/rio-scan.c | 139 ++- drivers/rapidio/rio-sysfs.c |4 +- drivers/rapidio/rio.c | 74 ++- drivers/rapidio/switches/idt_gen2.c | 93 --- drivers/rapidio/switches/idtcps.c |6 +- drivers/rapidio/switches/tsi568.c | 13 +-- drivers/rapidio/switches/tsi57x.c | 56 ++ include/linux/rio.h | 90 +++ include/linux/rio_drv.h | 72 +++--- 9 files changed, 214 insertions(+), 333 deletions(-) -- 1.7.3.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev