Hi Andrew, Am Mon, Jan 13, 2025 at 04:59:38PM +0100 schrieb Andrew Lunn: > On Mon, Jan 13, 2025 at 03:18:28PM +0100, Dimitri Fedrau wrote: > > Hi Andrew, > > > > Am Mon, Jan 13, 2025 at 02:54:28PM +0100 schrieb Andrew Lunn: > > > On Mon, Jan 13, 2025 at 06:40:11AM +0100, Dimitri Fedrau via B4 Relay wrote: > > > > Add support for changing the transmit amplitude voltage in 100BASE-TX mode. > > > > Add support for configuration via DT. > > > > > > The commit message is supposed to answer the question "Why?". Isn't > > > reducing the voltage going to make the device non conforming? Why > > > would i want to break it? I could understand setting it a bit higher > > > than required to handle losses on the PCB and connector, so the > > > voltages measured on the RJ45 pins are conforming. > > > > > - Will add the "Why?" to the commit description. You already answered it. > > - Yes you are right. > > - I don't want to break it, the PHY just provides these settings. And I > > just wanted to reflect this in the code, although it probably doesn't > > make sense. > > - In my case I want to set it a bit higher to be conforming. > > I have seen use cases for deeply embedded systems where they want to > reduce it, to avoid some EMC issues and save some power/heat. They > know the cable lengths, so know a lower voltage won't cause an > issue. The issue in that case is that the configuration was policy, > not a description of the hardware. So i pushed for it to be a PHY > tunable, which can be set at runtime. Your use case is however about > the hardware, you need a slightly higher voltage because of the > hardware design. So for this case, i think DT is O.K. But you will > need to make this clear in the commit message, you want to make the > device conform by increasing the voltage. And put something in the > binding description to indicate this setting should be used to tune > the PHY for conformance. It is not our problem if somebody misuses it > for EMC/power saving and makes there device non-conform. > Thanks for the explanation. Will add the comment in the commit message. > > > Also, what makes the dp8382 special? I know other PHYs can actually do > > > this. So why are we adding some vendor specific property just for > > > 100base-tx? > > > > > I don't think that the dp83822 is special in this case. I just didn't > > know better. Would be removing the vendor specific property enough ? > > Or is there already a defined property describing this. Didn't found > > anything. > > If i remember correctly, there might be a property for > automotive/safety critical, where there is a choice of two > voltages. But it might be just deciding which of the two voltages are > used? > > There is also: > > ti,cfg-dac-minus-one-bp: > description: | > DP83826 PHY only. > Sets the voltage ratio (with respect to the nominal value) > of the logical level -1 for the MLT-3 encoded TX data. > enum: [5000, 5625, 6250, 6875, 7500, 8125, 8750, 9375, 10000, > 10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000] > default: 10000 > > ti,cfg-dac-plus-one-bp: > description: | > DP83826 PHY only. > Sets the voltage ratio (with respect to the nominal value) > of the logical level +1 for the MLT-3 encoded TX data. > enum: [5000, 5625, 6250, 6875, 7500, 8125, 8750, 9375, 10000, > 10625, 11250, 11875, 12500, 13125, 13750, 14375, 15000] > default: 10000 > > I'm not so much an analogue person, but these don't make too much > sense to me. A ratio of 10,000 relative to nominal sounds rather > large. I would of expected a ration of 1 as the default? Since this > makes little sense to me, i don't think it is a good idea to copy it! > I think the ratio of 10.000 was chosen to avoid truncation when calculating register values. These are in steps of 6,25%. Using gain in DT seems to me the more generic way of describing amplitude modification. But converting the gain to register values might be more challenging in some cases. And I think we also need to scale when we want to represent it as gain. The DP83822 allows to modify the amplitude in terms of gain by 1.65% per step and for 10BASE-TE gain is 9% per step. I don't have any other PHY that allows me to modify the amplitude, so I'm quite biased. Do you know any other PHY supporting this ? > I've not looked at 802.3.... Do we need different settings for > different link modes? Are the losses dependent on the link mode? Are > the voltages different for different link modes? Is voltage actually > the best unit, if different link modes have different differential > voltages? Would a gain make more sense for a generic binding, and then > let the PHY driver convert that into whatever the hardware uses? > I had a quick look at 802.3, but I don't have much knowledge regarding the analogue part: - Different link modes have different differential voltages. - I would prefer gain instead of voltage, should also fit for fiber optics. - I would prefer separate settings for the link modes. Specs for the link modes are quite different, not that I could really judge. Maybe someone can help us out ? - Maybe something like tx-amplitude-100base-tx-gain-milli or tx-amplitude-100base-tx-gain-micro ? [...] Best regards, Dimitri Fedrau