| Message ID | 20210510051006.11393-2-a-govindraju@ti.com |
|---|---|
| State | Accepted |
| Commit | 307773f525eb9217090bd4b11748d880f7f99355 |
| Headers | show |
| Series | [v6,1/3] phy: core: Reword the comment specifying the units of max_link_rate to be Mbps | expand |
On 10-05-21, 10:40, Aswath Govindraju wrote: > In some subsystems (eg. CAN, SPI), the max link rate supported can be less > than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be > used. Therefore, leave the decision of units to be used, to the producer > and consumer. > > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > include/linux/phy/phy.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index 0ed434d02196..f3286f4cd306 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -125,7 +125,7 @@ struct phy_ops { > /** > * struct phy_attrs - represents phy attributes > * @bus_width: Data path width implemented by PHY > - * @max_link_rate: Maximum link rate supported by PHY (in Mbps) > + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer) So there are a few users of max_link_rate. It would be better that we document all previous users of max_link_rate that unit is in Mbps and then modify it here -- ~Vinod
Hi Vinod, On 31/05/21 12:48 pm, Vinod Koul wrote: > On 10-05-21, 10:40, Aswath Govindraju wrote: >> In some subsystems (eg. CAN, SPI), the max link rate supported can be less >> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be >> used. Therefore, leave the decision of units to be used, to the producer >> and consumer. >> >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> >> --- >> include/linux/phy/phy.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >> index 0ed434d02196..f3286f4cd306 100644 >> --- a/include/linux/phy/phy.h >> +++ b/include/linux/phy/phy.h >> @@ -125,7 +125,7 @@ struct phy_ops { >> /** >> * struct phy_attrs - represents phy attributes >> * @bus_width: Data path width implemented by PHY >> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps) >> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer) > > So there are a few users of max_link_rate. It would be better that we > document all previous users of max_link_rate that unit is in Mbps and > then modify it here > I was able to see that the max_link_rate attribute was used at, drivers/phy/cadence/phy-cadence-torrent.c:2514: gphy->attrs.max_link_rate = cdns_phy->max_bit_rate; and in the bindings there is indication that the units to be used is Mbps. Can you please point me if there is any other place that I might have missed to look at or that might need documentation update? Thanks, Aswath
Hi Vinod, On 31/05/21 2:04 pm, Aswath Govindraju wrote: > Hi Vinod, > > On 31/05/21 12:48 pm, Vinod Koul wrote: >> On 10-05-21, 10:40, Aswath Govindraju wrote: >>> In some subsystems (eg. CAN, SPI), the max link rate supported can be less >>> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be >>> used. Therefore, leave the decision of units to be used, to the producer >>> and consumer. >>> >>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> >>> --- >>> include/linux/phy/phy.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >>> index 0ed434d02196..f3286f4cd306 100644 >>> --- a/include/linux/phy/phy.h >>> +++ b/include/linux/phy/phy.h >>> @@ -125,7 +125,7 @@ struct phy_ops { >>> /** >>> * struct phy_attrs - represents phy attributes >>> * @bus_width: Data path width implemented by PHY >>> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps) >>> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer) >> >> So there are a few users of max_link_rate. It would be better that we >> document all previous users of max_link_rate that unit is in Mbps and >> then modify it here >> > > I was able to see that the max_link_rate attribute was used at, > > drivers/phy/cadence/phy-cadence-torrent.c:2514: > gphy->attrs.max_link_rate = cdns_phy->max_bit_rate; > > and in the bindings there is indication that the units to be used is Mbps. > > Can you please point me if there is any other place that I might have > missed to look at or that might need documentation update? > May I know if this patch series is good to be merged ? Thanks, Aswath > Thanks, > Aswath > >
Hi, On 11/06/21 6:12 pm, Aswath Govindraju wrote: > Hi Vinod, > > On 31/05/21 2:04 pm, Aswath Govindraju wrote: >> Hi Vinod, >> >> On 31/05/21 12:48 pm, Vinod Koul wrote: >>> On 10-05-21, 10:40, Aswath Govindraju wrote: >>>> In some subsystems (eg. CAN, SPI), the max link rate supported can be less >>>> than 1 Mbps and if the unit for max_link_rate is Mbps then it can't be >>>> used. Therefore, leave the decision of units to be used, to the producer >>>> and consumer. >>>> >>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >>>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> >>>> --- >>>> include/linux/phy/phy.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >>>> index 0ed434d02196..f3286f4cd306 100644 >>>> --- a/include/linux/phy/phy.h >>>> +++ b/include/linux/phy/phy.h >>>> @@ -125,7 +125,7 @@ struct phy_ops { >>>> /** >>>> * struct phy_attrs - represents phy attributes >>>> * @bus_width: Data path width implemented by PHY >>>> - * @max_link_rate: Maximum link rate supported by PHY (in Mbps) >>>> + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer) >>> >>> So there are a few users of max_link_rate. It would be better that we >>> document all previous users of max_link_rate that unit is in Mbps and >>> then modify it here >>> >> >> I was able to see that the max_link_rate attribute was used at, >> >> drivers/phy/cadence/phy-cadence-torrent.c:2514: >> gphy->attrs.max_link_rate = cdns_phy->max_bit_rate; >> >> and in the bindings there is indication that the units to be used is Mbps. >> >> Can you please point me if there is any other place that I might have >> missed to look at or that might need documentation update? >> The only user seems to be Torrent and max_bit_rate is documented well in Torrent. Acked-by: Kishon Vijay Abraham I <kishon@ti.com> Thanks Kishon
On 11-06-21, 18:31, Kishon Vijay Abraham I wrote: > >>> So there are a few users of max_link_rate. It would be better that we > >>> document all previous users of max_link_rate that unit is in Mbps and > >>> then modify it here > >>> > >> > >> I was able to see that the max_link_rate attribute was used at, > >> > >> drivers/phy/cadence/phy-cadence-torrent.c:2514: > >> gphy->attrs.max_link_rate = cdns_phy->max_bit_rate; > >> > >> and in the bindings there is indication that the units to be used is Mbps. > >> > >> Can you please point me if there is any other place that I might have > >> missed to look at or that might need documentation update? > >> > > The only user seems to be Torrent and max_bit_rate is documented well in > Torrent. > > Acked-by: Kishon Vijay Abraham I <kishon@ti.com> Series Applied, thanks -- ~Vinod
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index 0ed434d02196..f3286f4cd306 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -125,7 +125,7 @@ struct phy_ops { /** * struct phy_attrs - represents phy attributes * @bus_width: Data path width implemented by PHY - * @max_link_rate: Maximum link rate supported by PHY (in Mbps) + * @max_link_rate: Maximum link rate supported by PHY (units to be decided by producer and consumer) * @mode: PHY mode */ struct phy_attrs {