The tps23861 driver does not initialize the chip and relies on it being
in auto-mode by default. On some devices, these controllers default to
OFF-Mode and hence cannot be used at all.
This brings minimal support for initializing the controller in a user-
defined mode.
Signed-off-by: Andreas Böhler <[email protected]>
---
.../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
index 3bc8e73dfbf0..ed3a703478fb 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
@@ -35,6 +35,50 @@ required:
- compatible
- reg
+patternProperties:
+ "^port@([0-3])$":
+ type: object
+ description: Represents ports of the device and their specific configuration.
+
+ properties:
+ reg:
+ description: The port number
+ items:
+ minimum: 0
+ maximum: 3
+
+ mode:
+ description: The operating mode the device should be initialized with
+ items:
+ - enum:
+ - auto
+ - semiauto
+ - manual
+ - off
+
+ enable:
+ description: Whether the port should be enabled
+ items:
+ minimum: 0
+ maximum: 1
+
+ power:
+ description: Whether the port should be powered on
+ items:
+ minimum: 0
+ maximum: 1
+
+ poe_plus:
+ description: Whether the port should support PoE+
+ items:
+ minimum: 0
+ maximum: 1
+
+ required:
+ - reg
+
+ additionalProperties: false
+
additionalProperties: false
examples:
@@ -47,5 +91,37 @@ examples:
compatible = "ti,tps23861";
reg = <0x30>;
shunt-resistor-micro-ohms = <255000>;
+
+ port@0 {
+ reg = <0>;
+ mode = "auto";
+ enable = <1>;
+ power = <1>;
+ poe_plus = <1>;
+ };
+
+ port@1 {
+ reg = <1>;
+ mode = "semiauto";
+ enable = <1>;
+ power = <0>;
+ poe_plus = <1>;
+ };
+
+ port@2 {
+ reg = <2>;
+ mode = "manual";
+ enable = <0>;
+ power = <0>;
+ poe_plus = <0>;
+ };
+
+ port@3 {
+ reg = <3>;
+ mode = "off";
+ enable = <0>;
+ power = <0>;
+ poe_plus = <1>;
+ };
};
};
--
2.37.2
The tps23861 driver does not initialize the chip and relies on it being
in auto-mode by default. On some devices, these controllers default to
OFF-Mode and hence cannot be used at all.
This brings minimal support for initializing the controller in a user-
defined mode.
Tested on a TP-Link TL-SG2452P with 12x TI TPS23861 controllers.
Signed-off-by: Andreas Böhler <[email protected]>
---
drivers/hwmon/tps23861.c | 81 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/hwmon/tps23861.c b/drivers/hwmon/tps23861.c
index 42762e87b014..27bf8716cf12 100644
--- a/drivers/hwmon/tps23861.c
+++ b/drivers/hwmon/tps23861.c
@@ -85,6 +85,8 @@
#define PORT_DETECT_CAPACITANCE_INVALID_DELTA 11
#define PORT_DETECT_CAPACITANCE_OUT_OF_RANGE 12
#define POE_PLUS 0x40
+#define POE_PLUS_MASK(_port) \
+ GENMASK(_port + 4, _port + 4)
#define OPERATING_MODE 0x12
#define OPERATING_MODE_OFF 0
#define OPERATING_MODE_MANUAL 1
@@ -94,9 +96,22 @@
#define OPERATING_MODE_PORT_2_MASK GENMASK(3, 2)
#define OPERATING_MODE_PORT_3_MASK GENMASK(5, 4)
#define OPERATING_MODE_PORT_4_MASK GENMASK(7, 6)
+#define OPERATING_MODE_PORT(_mode, _port) \
+ (_mode << (_port * 2))
+#define DISCONNECT_ENABLE 0x13
+#define DISCONNECT_ENABLE_MASK(_port) \
+ GENMASK(_port, _port)
+#define DISCONNECT_MASK(_port) \
+ (GENMASK(_port, _port) | GENMASK(_port + 4, _port + 4))
+
+#define DETECT_CLASS_ENABLE 0x14
#define DETECT_CLASS_RESTART 0x18
#define POWER_ENABLE 0x19
+#define POWER_ENABLE_ON_MASK(_port) \
+ GENMASK(_port, _port)
+#define POWER_ENABLE_OFF_MASK(_port) \
+ GENMASK(_port + 4, _port + 4)
#define TPS23861_NUM_PORTS 4
#define TPS23861_GENERAL_MASK_1 0x17
@@ -548,7 +563,16 @@ static int tps23861_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct tps23861_data *data;
struct device *hwmon_dev;
+ struct device_node *child;
u32 shunt_resistor;
+ u32 reg;
+ u32 temp;
+ const char *mode;
+ unsigned int poe_plusval;
+ unsigned int mode_val;
+ unsigned int power_val;
+ unsigned int enable_val;
+ unsigned int disconnect_enable_val;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -577,6 +601,63 @@ static int tps23861_probe(struct i2c_client *client)
TPS23861_GENERAL_MASK_1,
TPS23861_CURRENT_SHUNT_MASK);
+ regmap_read(data->regmap, POE_PLUS, &poe_plusval);
+ regmap_read(data->regmap, POWER_ENABLE, &power_val);
+ regmap_read(data->regmap, OPERATING_MODE, &mode_val);
+ regmap_read(data->regmap, DETECT_CLASS_ENABLE, &enable_val);
+ regmap_read(data->regmap, DISCONNECT_ENABLE, &disconnect_enable_val);
+
+ for_each_child_of_node(dev->of_node, child) {
+ if (of_property_read_u32(child, "reg", ®))
+ continue;
+
+ if (reg > (TPS23861_NUM_PORTS - 1) || reg < 0)
+ continue;
+
+ if (!of_property_read_string(child, "mode", &mode)) {
+ if (!strncmp(mode, "manual", 6)) {
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_MANUAL, reg);
+ } else if (!strncmp(mode, "semiauto", 8)) {
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_SEMI, reg);
+ } else if (!strncmp(mode, "auto", 4))
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ else
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ }
+
+ if (!of_property_read_u32(child, "enable", &temp)) {
+ if (temp) {
+ enable_val |= DISCONNECT_MASK(reg);
+ disconnect_enable_val |= DISCONNECT_ENABLE_MASK(reg);
+ } else {
+ enable_val &= ~DISCONNECT_MASK(reg);
+ disconnect_enable_val &= ~DISCONNECT_ENABLE_MASK(reg);
+ }
+ }
+
+ if (!of_property_read_u32(child, "power", &temp)) {
+ if (temp)
+ power_val |= POWER_ENABLE_ON_MASK(reg);
+ else
+ power_val |= POWER_ENABLE_OFF_MASK(reg);
+ }
+
+ if (!of_property_read_u32(child, "poe_plus", &temp)) {
+ if (temp)
+ poe_plusval |= POE_PLUS_MASK(reg);
+ else
+ poe_plusval &= ~POE_PLUS_MASK(reg);
+ }
+ }
+
+ regmap_write(data->regmap, POE_PLUS, poe_plusval);
+ regmap_write(data->regmap, POWER_ENABLE, power_val);
+ regmap_write(data->regmap, OPERATING_MODE, mode_val);
+ regmap_write(data->regmap, DETECT_CLASS_ENABLE, enable_val);
+ regmap_write(data->regmap, DISCONNECT_ENABLE, disconnect_enable_val);
+
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
data, &tps23861_chip_info,
NULL);
--
2.37.2
On 25/08/2022 17:37, Andreas Böhler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
Thank you for your patch. There is something to discuss/improve.
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Signed-off-by: Andreas Böhler <[email protected]>
Use subject prefixes matching the subsystem (git log --oneline -- ...).
> ---
> .../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> index 3bc8e73dfbf0..ed3a703478fb 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> @@ -35,6 +35,50 @@ required:
> - compatible
> - reg
>
> +patternProperties:
> + "^port@([0-3])$":
No need for ()
> + type: object
> + description: Represents ports of the device and their specific configuration.
> +
> + properties:
> + reg:
> + description: The port number
> + items:
> + minimum: 0
> + maximum: 3
> +
> + mode:
> + description: The operating mode the device should be initialized with
> + items:
If this is real array, you need maxItems, but it looks one item, so no
need for "items".
> + - enum:
> + - auto
> + - semiauto
> + - manual
> + - off
And how "off" is different from disabled or powered off?
> +
> + enable:
> + description: Whether the port should be enabled
This looks confusing... Looks like boolean property, but instead you
define some numbers. What are the values for these numbers? Why these
are numbers not booleans?
Second - what does it mean "enable"? We have a generic "status" property
for it - isn't this the same?
Third, all these properties (especially PoE) look like you keep
describing here network device but this is HWMON part.
> + items:
> + minimum: 0
> + maximum: 1
> +
> + power:
> + description: Whether the port should be powered on
> + items:
> + minimum: 0
> + maximum: 1
> +
> + poe_plus:
No underscores in property names.
> + description: Whether the port should support PoE+
> + items:
> + minimum: 0
> + maximum: 1
> +
Best regards,
Krzysztof
On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas B?hler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Signed-off-by: Andreas B?hler <[email protected]>
nack for the series, sorry. The suggested properties are not hardware
monitoring but phy properties. There should be a separate phy driver
to manage those.
Also, as mentioned, the hwmon 'enable' attribute is abused to control
port functionality and should be removed.
Guenter
On Thu, Aug 25, 2022 at 5:02 PM Guenter Roeck <[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas Böhler wrote:
> > The tps23861 driver does not initialize the chip and relies on it being
> > in auto-mode by default. On some devices, these controllers default to
> > OFF-Mode and hence cannot be used at all.
> >
> > This brings minimal support for initializing the controller in a user-
> > defined mode.
> >
> > Signed-off-by: Andreas Böhler <[email protected]>
>
> nack for the series, sorry. The suggested properties are not hardware
> monitoring but phy properties. There should be a separate phy driver
> to manage those.
>
> Also, as mentioned, the hwmon 'enable' attribute is abused to control
> port functionality and should be removed.
Hi Guenter,
Are you referring to an ethernet PHY driver or the generic PHY framework?
Regards,
Robert
>
> Guenter
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Thu, Aug 25, 2022 at 05:07:45PM +0200, Robert Marko wrote:
> On Thu, Aug 25, 2022 at 5:02 PM Guenter Roeck <[email protected]> wrote:
> >
> > On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas B?hler wrote:
> > > The tps23861 driver does not initialize the chip and relies on it being
> > > in auto-mode by default. On some devices, these controllers default to
> > > OFF-Mode and hence cannot be used at all.
> > >
> > > This brings minimal support for initializing the controller in a user-
> > > defined mode.
> > >
> > > Signed-off-by: Andreas B?hler <[email protected]>
> >
> > nack for the series, sorry. The suggested properties are not hardware
> > monitoring but phy properties. There should be a separate phy driver
> > to manage those.
> >
> > Also, as mentioned, the hwmon 'enable' attribute is abused to control
> > port functionality and should be removed.
>
> Hi Guenter,
> Are you referring to an ethernet PHY driver or the generic PHY framework?
>
Could be both, though ethernet phy sounds about right for me.
I don't know where/how similar chips are handled. hwmon is most definitey
the wrong place.
Guenter
On Thu, Aug 25, 2022 at 5:29 PM Guenter Roeck <[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 05:07:45PM +0200, Robert Marko wrote:
> > On Thu, Aug 25, 2022 at 5:02 PM Guenter Roeck <[email protected]> wrote:
> > >
> > > On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas Böhler wrote:
> > > > The tps23861 driver does not initialize the chip and relies on it being
> > > > in auto-mode by default. On some devices, these controllers default to
> > > > OFF-Mode and hence cannot be used at all.
> > > >
> > > > This brings minimal support for initializing the controller in a user-
> > > > defined mode.
> > > >
> > > > Signed-off-by: Andreas Böhler <[email protected]>
> > >
> > > nack for the series, sorry. The suggested properties are not hardware
> > > monitoring but phy properties. There should be a separate phy driver
> > > to manage those.
> > >
> > > Also, as mentioned, the hwmon 'enable' attribute is abused to control
> > > port functionality and should be removed.
> >
> > Hi Guenter,
> > Are you referring to an ethernet PHY driver or the generic PHY framework?
> >
>
> Could be both, though ethernet phy sounds about right for me.
> I don't know where/how similar chips are handled. hwmon is most definitey
> the wrong place.
Hi,
Well, that is the thing, this is definitively not an ethernet PHY nor
a PHY of any other kind.
I dont see where it would fit if not hwmon, there is no more specific
subsystem in the
kernel.
Regards,
Robert
>
> Guenter
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr
On Thu, 25 Aug 2022 16:37:36 +0200, Andreas Böhler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Signed-off-by: Andreas Böhler <[email protected]>
> ---
> .../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:28.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:36.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@1:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:44.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@2:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:52.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@3:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:27.26-33.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@0: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:27.26-33.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@0: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:35.26-41.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@1: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:35.26-41.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@1: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:43.26-49.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@2: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:43.26-49.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@2: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:51.26-57.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@3: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:51.26-57.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@3: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On 25/08/2022 18:31, Robert Marko wrote:
> On Thu, Aug 25, 2022 at 5:29 PM Guenter Roeck <[email protected]> wrote:
>>
>> On Thu, Aug 25, 2022 at 05:07:45PM +0200, Robert Marko wrote:
>>> On Thu, Aug 25, 2022 at 5:02 PM Guenter Roeck <[email protected]> wrote:
>>>>
>>>> On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas Böhler wrote:
>>>>> The tps23861 driver does not initialize the chip and relies on it being
>>>>> in auto-mode by default. On some devices, these controllers default to
>>>>> OFF-Mode and hence cannot be used at all.
>>>>>
>>>>> This brings minimal support for initializing the controller in a user-
>>>>> defined mode.
>>>>>
>>>>> Signed-off-by: Andreas Böhler <[email protected]>
>>>>
>>>> nack for the series, sorry. The suggested properties are not hardware
>>>> monitoring but phy properties. There should be a separate phy driver
>>>> to manage those.
>>>>
>>>> Also, as mentioned, the hwmon 'enable' attribute is abused to control
>>>> port functionality and should be removed.
>>>
>>> Hi Guenter,
>>> Are you referring to an ethernet PHY driver or the generic PHY framework?
>>>
>>
>> Could be both, though ethernet phy sounds about right for me.
>> I don't know where/how similar chips are handled. hwmon is most definitey
>> the wrong place.
>
> Hi,
>
> Well, that is the thing, this is definitively not an ethernet PHY nor
> a PHY of any other kind.
> I dont see where it would fit if not hwmon, there is no more specific
> subsystem in the
> kernel.
It's not hwmon. The device has monitoring capabilities, but it's only
one piece and calling something hwmon just because can provide sensor
data is like calling a plane a car, because it has wheels.
Maybe this is similar to these series:
https://lore.kernel.org/linux-devicetree/[email protected]/
?
The datasheet says it is a "PSE Controller" so looks similar to the
problem solved above...
Best regards,
Krzysztof
On Fri, Aug 26, 2022 at 09:56:29AM +0300, Krzysztof Kozlowski wrote:
> On 25/08/2022 18:31, Robert Marko wrote:
> > On Thu, Aug 25, 2022 at 5:29 PM Guenter Roeck <[email protected]> wrote:
> >>
> >> On Thu, Aug 25, 2022 at 05:07:45PM +0200, Robert Marko wrote:
> >>> On Thu, Aug 25, 2022 at 5:02 PM Guenter Roeck <[email protected]> wrote:
> >>>>
> >>>> On Thu, Aug 25, 2022 at 04:37:36PM +0200, Andreas B?hler wrote:
> >>>>> The tps23861 driver does not initialize the chip and relies on it being
> >>>>> in auto-mode by default. On some devices, these controllers default to
> >>>>> OFF-Mode and hence cannot be used at all.
> >>>>>
> >>>>> This brings minimal support for initializing the controller in a user-
> >>>>> defined mode.
> >>>>>
> >>>>> Signed-off-by: Andreas B?hler <[email protected]>
> >>>>
> >>>> nack for the series, sorry. The suggested properties are not hardware
> >>>> monitoring but phy properties. There should be a separate phy driver
> >>>> to manage those.
> >>>>
> >>>> Also, as mentioned, the hwmon 'enable' attribute is abused to control
> >>>> port functionality and should be removed.
> >>>
> >>> Hi Guenter,
> >>> Are you referring to an ethernet PHY driver or the generic PHY framework?
> >>>
> >>
> >> Could be both, though ethernet phy sounds about right for me.
> >> I don't know where/how similar chips are handled. hwmon is most definitey
> >> the wrong place.
> >
> > Hi,
> >
> > Well, that is the thing, this is definitively not an ethernet PHY nor
> > a PHY of any other kind.
> > I dont see where it would fit if not hwmon, there is no more specific
> > subsystem in the
> > kernel.
>
> It's not hwmon. The device has monitoring capabilities, but it's only
> one piece and calling something hwmon just because can provide sensor
> data is like calling a plane a car, because it has wheels.
>
> Maybe this is similar to these series:
> https://lore.kernel.org/linux-devicetree/[email protected]/
> ?
>
> The datasheet says it is a "PSE Controller" so looks similar to the
> problem solved above...
Excellent find. That infrastructure is exactly what the driver for this chip
needs to tie into. I would suggest to get in touch with the author of that
series - it is quite likely that they are working on adding support for one
or more real PSE chips.
The only open question is if the hwmon driver should be retained as a
separate driver or be implemented as part of the PSE networking driver.
I am open to both.
Thanks,
Guenter