diff mbox

[1/3] ipmi/bt-bmc: change compatible node to 'aspeed, ast2400-ibt-bmc'

Message ID 1478073426-3714-2-git-send-email-clg@kaod.org
State Not Applicable, archived
Headers show

Commit Message

Cédric Le Goater Nov. 2, 2016, 7:57 a.m. UTC
The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
other is H8S/2168 compliant.

The current ipmi/bt-bmc driver implements the IPMI version and we
should reflect its nature in the compatible node name using
'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
latter should be used for a H8S interface driver if it is implemented
one day.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 .../ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt}    | 4 ++--
 drivers/char/ipmi/bt-bmc.c                                            | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename Documentation/devicetree/bindings/ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt} (85%)

Comments

Arnd Bergmann Nov. 2, 2016, 1:15 p.m. UTC | #1
On Wednesday 02 November 2016, Cédric Le Goater wrote:
> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
> other is H8S/2168 compliant.
> 
> The current ipmi/bt-bmc driver implements the IPMI version and we
> should reflect its nature in the compatible node name using
> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
> latter should be used for a H8S interface driver if it is implemented
> one day.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

We generally try to avoid changing the compatible strings after the
fact, but it's probably ok in this case.

I don't understand who decides which of the two interfaces is used:
is it the same register set that can be driven by either one or the
other driver, or do you expect to have two drivers that can both
be active in the same system and talk to different hardware once
you get there?

If the first one of these is true, it seems a little awkward to
use the DT compatible string to decide which driver to use rather
than making the decision in the OS.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joel Stanley Nov. 2, 2016, 1:56 p.m. UTC | #2
On Wed, Nov 2, 2016 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 02 November 2016, Cédric Le Goater wrote:
>> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
>> other is H8S/2168 compliant.
>>
>> The current ipmi/bt-bmc driver implements the IPMI version and we
>> should reflect its nature in the compatible node name using
>> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
>> latter should be used for a H8S interface driver if it is implemented
>> one day.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
> We generally try to avoid changing the compatible strings after the
> fact, but it's probably ok in this case.
>
> I don't understand who decides which of the two interfaces is used:
> is it the same register set that can be driven by either one or the
> other driver, or do you expect to have two drivers that can both
> be active in the same system and talk to different hardware once
> you get there?

It's the second case. The H8S BT has a different register layout so it
would require a different driver.

We don't yet have a driver for the other BT device, but there was
recent talk of using it as an alternate (non-ipmi channel) between the
BMC and the host. Before that discussion I wasn't aware that the H8S
BT existed. I suggested we fix this up before it hits a final release.

Cédric, do you think ast2400-ibt-bmc or ast2400-ipmi-bt-bmc does a
better job of describing the hardware here?

While we're modifying the binding, should we add a compat string for
the ast2500?

Cheers,

Joel

>
> If the first one of these is true, it seems a little awkward to
> use the DT compatible string to decide which driver to use rather
> than making the decision in the OS.
>
>         Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cédric Le Goater Nov. 2, 2016, 2:28 p.m. UTC | #3
On 11/02/2016 02:56 PM, Joel Stanley wrote:
> On Wed, Nov 2, 2016 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday 02 November 2016, Cédric Le Goater wrote:
>>> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
>>> other is H8S/2168 compliant.
>>>
>>> The current ipmi/bt-bmc driver implements the IPMI version and we
>>> should reflect its nature in the compatible node name using
>>> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
>>> latter should be used for a H8S interface driver if it is implemented
>>> one day.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>
>> We generally try to avoid changing the compatible strings after the
>> fact, but it's probably ok in this case.

As the device tree changes are not merged yet, we thought we had some 
more time to fine tune the naming. 
 
>> I don't understand who decides which of the two interfaces is used:
>> is it the same register set that can be driven by either one or the
>> other driver, or do you expect to have two drivers that can both
>> be active in the same system and talk to different hardware once
>> you get there?
> 
> It's the second case. The H8S BT has a different register layout so it
> would require a different driver.

yes.
 
> We don't yet have a driver for the other BT device, but there was
> recent talk of using it as an alternate (non-ipmi channel) between the
> BMC and the host. Before that discussion I wasn't aware that the H8S
> BT existed. I suggested we fix this up before it hits a final release.
> 
> Cédric, do you think ast2400-ibt-bmc or ast2400-ipmi-bt-bmc does a
> better job of describing the hardware here?

The specs refer to the two interfaces as BT (non IPMI) and iBT (IPMI). 
I think we can keep the same naming.

> While we're modifying the binding, should we add a compat string for
> the ast2500?

Well, if the change in this patch is fine for all, may be we can add 
the ast2500 compat string in a followup patch ?

Thanks,

C. 

> Cheers,
> 
> Joel
> 
>>
>> If the first one of these is true, it seems a little awkward to
>> use the DT compatible string to decide which driver to use rather
>> than making the decision in the OS.
>>
>>         Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Nov. 7, 2016, 1:02 p.m. UTC | #4
On Wednesday, November 2, 2016 3:28:01 PM CET Cédric Le Goater wrote:
> On 11/02/2016 02:56 PM, Joel Stanley wrote:
> > On Wed, Nov 2, 2016 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Wednesday 02 November 2016, Cédric Le Goater wrote:
> >>> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
> >>> other is H8S/2168 compliant.
> >>>
> >>> The current ipmi/bt-bmc driver implements the IPMI version and we
> >>> should reflect its nature in the compatible node name using
> >>> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
> >>> latter should be used for a H8S interface driver if it is implemented
> >>> one day.
> >>>
> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>
> >> We generally try to avoid changing the compatible strings after the
> >> fact, but it's probably ok in this case.
> 
> As the device tree changes are not merged yet, we thought we had some 
> more time to fine tune the naming. 

Ok, I see. No problem then.

> >> I don't understand who decides which of the two interfaces is used:
> >> is it the same register set that can be driven by either one or the
> >> other driver, or do you expect to have two drivers that can both
> >> be active in the same system and talk to different hardware once
> >> you get there?
> > 
> > It's the second case. The H8S BT has a different register layout so it
> > would require a different driver.
> 
> yes.
>  
> > We don't yet have a driver for the other BT device, but there was
> > recent talk of using it as an alternate (non-ipmi channel) between the
> > BMC and the host. Before that discussion I wasn't aware that the H8S
> > BT existed. I suggested we fix this up before it hits a final release.
> > 
> > Cédric, do you think ast2400-ibt-bmc or ast2400-ipmi-bt-bmc does a
> > better job of describing the hardware here?
> 
> The specs refer to the two interfaces as BT (non IPMI) and iBT (IPMI). 
> I think we can keep the same naming.

Ok

> > While we're modifying the binding, should we add a compat string for
> > the ast2500?
> 
> Well, if the change in this patch is fine for all, may be we can add 
> the ast2500 compat string in a followup patch ?

Sounds good to me.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cédric Le Goater Nov. 8, 2016, 3:52 p.m. UTC | #5
On 11/07/2016 02:02 PM, Arnd Bergmann wrote:
> On Wednesday, November 2, 2016 3:28:01 PM CET Cédric Le Goater wrote:
>> On 11/02/2016 02:56 PM, Joel Stanley wrote:
>>> On Wed, Nov 2, 2016 at 11:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> On Wednesday 02 November 2016, Cédric Le Goater wrote:
>>>>> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
>>>>> other is H8S/2168 compliant.
>>>>>
>>>>> The current ipmi/bt-bmc driver implements the IPMI version and we
>>>>> should reflect its nature in the compatible node name using
>>>>> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
>>>>> latter should be used for a H8S interface driver if it is implemented
>>>>> one day.
>>>>>
>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>
>>>> We generally try to avoid changing the compatible strings after the
>>>> fact, but it's probably ok in this case.
>>
>> As the device tree changes are not merged yet, we thought we had some 
>> more time to fine tune the naming. 
> 
> Ok, I see. No problem then.
> 
>>>> I don't understand who decides which of the two interfaces is used:
>>>> is it the same register set that can be driven by either one or the
>>>> other driver, or do you expect to have two drivers that can both
>>>> be active in the same system and talk to different hardware once
>>>> you get there?
>>>
>>> It's the second case. The H8S BT has a different register layout so it
>>> would require a different driver.
>>
>> yes.
>>  
>>> We don't yet have a driver for the other BT device, but there was
>>> recent talk of using it as an alternate (non-ipmi channel) between the
>>> BMC and the host. Before that discussion I wasn't aware that the H8S
>>> BT existed. I suggested we fix this up before it hits a final release.
>>>
>>> Cédric, do you think ast2400-ibt-bmc or ast2400-ipmi-bt-bmc does a
>>> better job of describing the hardware here?
>>
>> The specs refer to the two interfaces as BT (non IPMI) and iBT (IPMI). 
>> I think we can keep the same naming.
> 
> Ok
> 
>>> While we're modifying the binding, should we add a compat string for
>>> the ast2500?
>>
>> Well, if the change in this patch is fine for all, may be we can add 
>> the ast2500 compat string in a followup patch ?
> 
> Sounds good to me.

OK. So, how do we proceed with this patch ? Who would include it in its 
tree ? 

Thanks,

C.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Corey Minyard Nov. 8, 2016, 6:15 p.m. UTC | #6
On 11/08/2016 09:52 AM, Cédric Le Goater wrote:
> O
snip

>>>> While we're modifying the binding, should we add a compat string for
>>>> the ast2500?
>>> Well, if the change in this patch is fine for all, may be we can add
>>> the ast2500 compat string in a followup patch ?
>> Sounds good to me.
> OK. So, how do we proceed with this patch ? Who would include it in its
> tree ?

I don't have anything for 4.9 at the moment.  Arnd, if you have 
something, can
you take this?  Otherwise I will.

And I guess I should add:

Acked-by: Corey Minyard <cminyard@mvista.com>

-corey

> Thanks,
>
> C.
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Nov. 9, 2016, 4:09 p.m. UTC | #7
On Tuesday, November 8, 2016 12:15:57 PM CET Corey Minyard wrote:
> On 11/08/2016 09:52 AM, Cédric Le Goater wrote:
> > O
> snip
> 
> >>>> While we're modifying the binding, should we add a compat string for
> >>>> the ast2500?
> >>> Well, if the change in this patch is fine for all, may be we can add
> >>> the ast2500 compat string in a followup patch ?
> >> Sounds good to me.
> > OK. So, how do we proceed with this patch ? Who would include it in its
> > tree ?
> 
> I don't have anything for 4.9 at the moment.  Arnd, if you have 
> something, can
> you take this?  Otherwise I will.
> 
> And I guess I should add:
> 
> Acked-by: Corey Minyard <cminyard@mvista.com>

Thanks, I've added it to my todo folder.

Olof, if you do fixes before I do, please pick up this patch with
Corey's Ack.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Nov. 9, 2016, 6:26 p.m. UTC | #8
On Wed, Nov 02, 2016 at 08:57:04AM +0100, Cédric Le Goater wrote:
> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
> other is H8S/2168 compliant.
> 
> The current ipmi/bt-bmc driver implements the IPMI version and we
> should reflect its nature in the compatible node name using
> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
> latter should be used for a H8S interface driver if it is implemented
> one day.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  .../ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt}    | 4 ++--

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/char/ipmi/bt-bmc.c                                            | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>  rename Documentation/devicetree/bindings/ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt} (85%)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joel Stanley Nov. 10, 2016, 2:49 a.m. UTC | #9
On Thu, Nov 10, 2016 at 2:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, November 8, 2016 12:15:57 PM CET Corey Minyard wrote:
>> On 11/08/2016 09:52 AM, Cédric Le Goater wrote:
>> > O
>> snip
>>
>> >>>> While we're modifying the binding, should we add a compat string for
>> >>>> the ast2500?
>> >>> Well, if the change in this patch is fine for all, may be we can add
>> >>> the ast2500 compat string in a followup patch ?
>> >> Sounds good to me.
>> > OK. So, how do we proceed with this patch ? Who would include it in its
>> > tree ?
>>
>> I don't have anything for 4.9 at the moment.  Arnd, if you have
>> something, can
>> you take this?  Otherwise I will.
>>
>> And I guess I should add:
>>
>> Acked-by: Corey Minyard <cminyard@mvista.com>
>
> Thanks, I've added it to my todo folder.
>
> Olof, if you do fixes before I do, please pick up this patch with
> Corey's Ack.

Thank you!

Acked-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson Nov. 18, 2016, 12:33 a.m. UTC | #10
On Wed, Nov 09, 2016 at 05:09:12PM +0100, Arnd Bergmann wrote:
> On Tuesday, November 8, 2016 12:15:57 PM CET Corey Minyard wrote:
> > On 11/08/2016 09:52 AM, C?dric Le Goater wrote:
> > > O
> > snip
> > 
> > >>>> While we're modifying the binding, should we add a compat string for
> > >>>> the ast2500?
> > >>> Well, if the change in this patch is fine for all, may be we can add
> > >>> the ast2500 compat string in a followup patch ?
> > >> Sounds good to me.
> > > OK. So, how do we proceed with this patch ? Who would include it in its
> > > tree ?
> > 
> > I don't have anything for 4.9 at the moment.  Arnd, if you have 
> > something, can
> > you take this?  Otherwise I will.
> > 
> > And I guess I should add:
> > 
> > Acked-by: Corey Minyard <cminyard@mvista.com>
> 
> Thanks, I've added it to my todo folder.
> 
> Olof, if you do fixes before I do, please pick up this patch with
> Corey's Ack.

Done, applied to fixes.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-bt-bmc.txt b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.txt
similarity index 85%
rename from Documentation/devicetree/bindings/ipmi/aspeed,ast2400-bt-bmc.txt
rename to Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.txt
index fbbacd958240..6f28969af9dc 100644
--- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-bt-bmc.txt
+++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-ibt-bmc.txt
@@ -6,7 +6,7 @@  perform in-band IPMI communication with their host.
 
 Required properties:
 
-- compatible : should be "aspeed,ast2400-bt-bmc"
+- compatible : should be "aspeed,ast2400-ibt-bmc"
 - reg: physical address and size of the registers
 
 Optional properties:
@@ -17,7 +17,7 @@  Optional properties:
 Example:
 
 	ibt@1e789140 {
-		compatible = "aspeed,ast2400-bt-bmc";
+		compatible = "aspeed,ast2400-ibt-bmc";
 		reg = <0x1e789140 0x18>;
 		interrupts = <8>;
 	};
diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index b49e61320952..fc9e8891eae3 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -484,7 +484,7 @@  static int bt_bmc_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id bt_bmc_match[] = {
-	{ .compatible = "aspeed,ast2400-bt-bmc" },
+	{ .compatible = "aspeed,ast2400-ibt-bmc" },
 	{ },
 };
 
@@ -502,4 +502,4 @@  module_platform_driver(bt_bmc_driver);
 MODULE_DEVICE_TABLE(of, bt_bmc_match);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Alistair Popple <alistair@popple.id.au>");
-MODULE_DESCRIPTION("Linux device interface to the BT interface");
+MODULE_DESCRIPTION("Linux device interface to the IPMI BT interface");